This commit is contained in:
Demi Marie Obenour 2026-05-24 17:37:38 -04:00 committed by GitHub
commit 9c9031b57a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 309 additions and 4 deletions

View file

@ -688,11 +688,18 @@ ngx_int_t
ngx_parse_url(ngx_pool_t *pool, ngx_url_t *u)
{
u_char *p;
size_t len;
size_t len, i;
p = u->url.data;
len = u->url.len;
for (i = 0; i < len; ++i) {
if (p[i] <= 0x20 || p[i] == 0x7F) {
u->err = "Control character in URL (possible CRLF injection attack?)";
return NGX_ERROR;
}
}
if (len >= 5 && ngx_strncasecmp(p, (u_char *) "unix:", 5) == 0) {
return ngx_parse_unix_domain_url(pool, u);
}

View file

@ -252,6 +252,14 @@ ngx_http_headers_filter(ngx_http_request_t *r)
return NGX_ERROR;
}
if (ngx_http_valid_header_value(value) != NGX_OK) {
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
"Header value contains forbidden characters "
"(do you use variables in header values that "
"can contain CR or LF?)");
return NGX_ERROR;
}
if (h[i].handler(r, &h[i], &value) != NGX_OK) {
return NGX_ERROR;
}
@ -337,6 +345,14 @@ ngx_http_trailers_filter(ngx_http_request_t *r, ngx_chain_t *in)
}
if (value.len) {
if (ngx_http_valid_header_value(value) != NGX_OK) {
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
"Header value contains forbidden characters "
"(do you use variables in header values that "
"can contain CR or LF?)");
return NGX_ERROR;
}
t = ngx_list_push(&r->headers_out.trailers);
if (t == NULL) {
return NGX_ERROR;
@ -851,6 +867,11 @@ ngx_http_headers_add(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
value = cf->args->elts;
if (ngx_conf_check_field_name_and_strip_value(cf, value, value + 1,
value + 2) != NGX_OK) {
return NGX_CONF_ERROR;
}
headers = (ngx_array_t **) ((char *) hcf + cmd->offset);
if (*headers == NULL) {

View file

@ -108,6 +108,8 @@ static char *ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf,
static ngx_int_t ngx_http_proxy_init_headers(ngx_conf_t *cf,
ngx_http_proxy_loc_conf_t *conf, ngx_http_proxy_headers_t *headers,
ngx_keyval_t *default_headers);
static char *ngx_http_proxy_set_header(ngx_conf_t *cf,
ngx_command_t *cmd, void *conf);
static char *ngx_http_proxy_pass(ngx_conf_t *cf, ngx_command_t *cmd,
void *conf);
@ -321,7 +323,7 @@ static ngx_command_t ngx_http_proxy_commands[] = {
{ ngx_string("proxy_set_header"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
ngx_conf_set_keyval_slot,
ngx_http_proxy_set_header,
NGX_HTTP_LOC_CONF_OFFSET,
offsetof(ngx_http_proxy_loc_conf_t, headers_source),
NULL },
@ -1179,7 +1181,7 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
key_len, val_len;
uintptr_t escape;
ngx_buf_t *b;
ngx_str_t method;
ngx_str_t method, tmp;
ngx_uint_t i, unparsed_uri;
ngx_chain_t *cl, *body;
ngx_list_part_t *part;
@ -1329,6 +1331,15 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
continue;
}
if (ngx_http_valid_header_name(header[i].key) != NGX_OK
|| ngx_http_valid_header_value(header[i].value) != NGX_OK) {
/* oops, request smuggling */
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Internal malformed header name or value "
"detected (this is a bug!!!!");
return NGX_ERROR;
}
len += header[i].key.len + sizeof(": ") - 1
+ header[i].value.len + sizeof(CRLF) - 1;
}
@ -1346,6 +1357,7 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
}
cl->buf = b;
cl->next = NULL;
/* the request line */
@ -1384,6 +1396,15 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
u->uri.len = b->last - u->uri.data;
for (uri_len = 0; uri_len < u->uri.len; ++uri_len) {
if (u->uri.data[uri_len] <= 0x20 || u->uri.data[uri_len] == 0x7F) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"URI contains control characters "
"(possible CRLF injection?");
return NGX_ERROR;
}
}
if (plcf->http_version == NGX_HTTP_VERSION_11) {
b->last = ngx_cpymem(b->last, ngx_http_proxy_version_11,
sizeof(ngx_http_proxy_version_11) - 1);
@ -1404,6 +1425,7 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
while (*(uintptr_t *) le.ip) {
tmp.data = e.pos;
lcode = *(ngx_http_script_len_code_pt *) le.ip;
(void) lcode(&le);
@ -1428,14 +1450,30 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
code = *(ngx_http_script_code_pt *) e.ip;
code((ngx_http_script_engine_t *) &e);
tmp.len = (size_t)(e.pos - tmp.data);
if (ngx_http_valid_header_name(tmp) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header name contains forbidden characters "
"(do you use header names with variables?)");
return NGX_ERROR;
}
*e.pos++ = ':'; *e.pos++ = ' ';
tmp.data = e.pos;
while (*(uintptr_t *) e.ip) {
code = *(ngx_http_script_code_pt *) e.ip;
code((ngx_http_script_engine_t *) &e);
}
e.ip += sizeof(uintptr_t);
tmp.len = (size_t)(e.pos - tmp.data);
if (ngx_http_valid_header_value(tmp) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header value contains forbidden characters "
"(do you use variables in header values that "
"can contain CR or LF?)");
return NGX_ERROR;
}
*e.pos++ = CR; *e.pos++ = LF;
}
@ -4118,6 +4156,22 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
return NGX_CONF_OK;
}
static char *
ngx_http_proxy_set_header(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
{
char *rc;
ngx_str_t *value;
value = cf->args->elts;
if (ngx_conf_check_field_name_and_strip_value(cf, value, value + 1,
value + 2) != NGX_OK) {
return NGX_CONF_ERROR;
}
rc = ngx_conf_set_keyval_slot(cf, cmd, conf);
return rc;
}
static ngx_int_t
ngx_http_proxy_init_headers(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *conf,

View file

@ -322,7 +322,7 @@ ngx_http_proxy_v2_create_request(ngx_http_request_t *r)
loc_len, body_len;
uintptr_t escape;
ngx_buf_t *b;
ngx_str_t method, *host;
ngx_str_t method, *host, tmp_str;
ngx_uint_t i, next, unparsed_uri;
ngx_chain_t *cl, *body;
ngx_list_part_t *part;
@ -517,6 +517,16 @@ ngx_http_proxy_v2_create_request(ngx_http_request_t *r)
continue;
}
if (ngx_http_valid_header_name(header[i].key) != NGX_OK
|| ngx_http_valid_header_value(header[i].value) != NGX_OK) {
/* oops, request smuggling */
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Internal malformed header name or value "
"detected (this is a bug!!!!");
return NGX_ERROR;
}
len += 1 + NGX_HTTP_V2_INT_OCTETS + header[i].key.len
+ NGX_HTTP_V2_INT_OCTETS + header[i].value.len;
@ -714,6 +724,15 @@ ngx_http_proxy_v2_create_request(ngx_http_request_t *r)
code = *(ngx_http_script_code_pt *) e.ip;
code((ngx_http_script_engine_t *) &e);
tmp_str.data = key_tmp;
tmp_str.len = key_len;
if (ngx_http_valid_header_name(tmp_str) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header name contains forbidden characters "
"(do you use header names with variables?)");
return NGX_ERROR;
}
b->last = ngx_http_v2_write_name(b->last, key_tmp, key_len, tmp);
e.pos = val_tmp;
@ -724,6 +743,16 @@ ngx_http_proxy_v2_create_request(ngx_http_request_t *r)
}
e.ip += sizeof(uintptr_t);
tmp_str.data = val_tmp;
tmp_str.len = val_len;
if (ngx_http_valid_header_value(tmp_str) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header value contains forbidden characters "
"(do you use variables in header values that "
"can contain CR or LF?)");
return NGX_ERROR;
}
b->last = ngx_http_v2_write_value(b->last, val_tmp, val_len, tmp);
#if (NGX_DEBUG)

View file

@ -2198,3 +2198,77 @@ ngx_http_set_default_types(ngx_conf_t *cf, ngx_array_t **types,
return NGX_OK;
}
ngx_int_t
ngx_conf_check_field_name_and_strip_value(ngx_conf_t *cf,
const ngx_str_t *cmd_name, const ngx_str_t *name, ngx_str_t *value)
{
size_t out_cursor = 0, cursor = 0, end, original_end;
u_char *ptr, ch;
if (ngx_http_valid_header_name(*name) != NGX_OK) {
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
"%V: Invalid HTTP field name",
(ngx_str_t *)cmd_name);
return NGX_ERROR;
}
#define ISSPACE(c) ((c) == ' ' || (c) == '\t')
end = original_end = value->len;
ptr = value->data;
while (cursor < end && ISSPACE(ptr[cursor])) {
cursor++;
}
for ( /* void */; cursor < end; ++cursor) {
ch = ptr[cursor];
if (ch == 0) {
goto fail;
}
if (ch != CR && ch != LF) {
ptr[out_cursor++] = ch;
continue;
}
if (ch == CR) {
if (end - cursor < 2 || ptr[cursor + 1] != LF) {
goto fail;
}
cursor++;
}
cursor++;
if (cursor >= end || !ISSPACE(ptr[cursor])) {
goto fail;
}
do {
cursor++;
} while (cursor < end && ISSPACE(ptr[cursor]));
while (out_cursor > 0 && ISSPACE(ptr[out_cursor - 1])) {
out_cursor--;
}
if (out_cursor > 0) {
ptr[out_cursor++] = ' ';
}
}
while (out_cursor > 0 && ISSPACE(ptr[out_cursor - 1])) {
out_cursor--;
}
if (out_cursor < original_end) {
memset(ptr + out_cursor, '\0', (size_t)(original_end - out_cursor));
}
value->len = out_cursor;
return NGX_OK;
fail:
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
"%V: Invalid HTTP field value", (ngx_str_t *)cmd_name);
return NGX_ERROR;
}

View file

@ -122,6 +122,15 @@ void ngx_http_split_args(ngx_http_request_t *r, ngx_str_t *uri,
ngx_int_t ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
ngx_http_chunked_t *ctx, ngx_uint_t keep_trailers);
/** Check if value is a valid HTTP field name. */
ngx_int_t ngx_http_valid_header_name(ngx_str_t value);
/** Check if value is a valid HTTP field value. */
ngx_int_t ngx_http_valid_header_value(ngx_str_t value);
/** Check if the field name and value are valid for an HTTP field name-value
* pair. Leading and trailing ' ' and '\t' are stripped from the field value
* before the value is checked. The value is updated in-place. */
ngx_int_t ngx_conf_check_field_name_and_strip_value(ngx_conf_t *cf,
const ngx_str_t *cmd_name, const ngx_str_t *name, ngx_str_t *value);
ngx_http_request_t *ngx_http_create_request(ngx_connection_t *c);
ngx_int_t ngx_http_process_request_uri(ngx_http_request_t *r);

View file

@ -435,6 +435,21 @@ ngx_http_header_filter(ngx_http_request_t *r)
continue;
}
if (ngx_http_valid_header_name(header[i].key) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header name contains forbidden characters "
"(do you use header names with variables?)");
return NGX_ERROR;
}
if (ngx_http_valid_header_value(header[i].value) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header name contains forbidden characters "
"(do you use variables in header values that "
"can contain CR or LF?)");
return NGX_ERROR;
}
len += header[i].key.len + sizeof(": ") - 1 + header[i].value.len
+ sizeof(CRLF) - 1;
}
@ -603,6 +618,19 @@ ngx_http_header_filter(ngx_http_request_t *r)
continue;
}
if (ngx_http_valid_header_name(header[i].key) != NGX_OK) {
ngx_log_error(NGX_LOG_EMERG, r->connection->log, 0,
"BUG: internal bad header name not caught earlier");
return NGX_ERROR;
}
if (ngx_http_valid_header_value(header[i].value) != NGX_OK) {
ngx_log_error(NGX_LOG_EMERG, r->connection->log, 0,
"BUG: internal bad header value not caught earlier");
return NGX_ERROR;
}
b->last = ngx_copy(b->last, header[i].key.data, header[i].key.len);
*b->last++ = ':'; *b->last++ = ' ';
@ -668,6 +696,21 @@ ngx_http_early_hints_filter(ngx_http_request_t *r)
continue;
}
if (ngx_http_valid_header_name(header[i].key) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header name contains forbidden characters "
"(do you use header names with variables?)");
return NGX_ERROR;
}
if (ngx_http_valid_header_value(header[i].value) != NGX_OK) {
ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
"Header name contains forbidden characters "
"(do you use variables in header values that "
"can contain CR or LF?)");
return NGX_ERROR;
}
len += header[i].key.len + sizeof(": ") - 1 + header[i].value.len
+ sizeof(CRLF) - 1;
}
@ -707,6 +750,18 @@ ngx_http_early_hints_filter(ngx_http_request_t *r)
continue;
}
if (ngx_http_valid_header_name(header[i].key) != NGX_OK) {
ngx_log_error(NGX_LOG_EMERG, r->connection->log, 0,
"BUG: internal bad header name not caught earlier");
return NGX_ERROR;
}
if (ngx_http_valid_header_value(header[i].value) != NGX_OK) {
ngx_log_error(NGX_LOG_EMERG, r->connection->log, 0,
"BUG: internal bad header value not caught earlier");
return NGX_ERROR;
}
b->last = ngx_copy(b->last, header[i].key.data, header[i].key.len);
*b->last++ = ':'; *b->last++ = ' ';

View file

@ -2483,3 +2483,59 @@ invalid:
return NGX_ERROR;
}
ngx_int_t
ngx_http_valid_header_value(ngx_str_t value)
{
size_t j;
/* TODO: this might break configurations that are invalid,
* but happen to work right now because the leading and/or
* trailing whitespace gets stripped. It should be turned on
* if NGINX ever has a major version bump, or if a mechanism
* is added for the user to opt-in. Sending a message with
* leading or trailing space is much less serious than CRLF
* injection. */
if (0) {
if (value.len < 1) {
return NGX_OK;
}
if (value.data[0] == ' ' || value.data[0] == '\t'
|| value.data[value.len - 1] == ' '
|| value.data[value.len - 1] == '\t') {
return NGX_ERROR;
}
}
for (j = 0; j < value.len; ++j) {
if (value.data[j] == '\0' || value.data[j] == CR
|| value.data[j] == LF) {
return NGX_ERROR;
}
}
return NGX_OK;
}
ngx_int_t
ngx_http_valid_header_name(ngx_str_t value)
{
size_t j;
if (value.len < 1) {
return NGX_ERROR;
}
for (j = 0; j < value.len; ++j) {
/* todo: check that this is an HTTP TOKEN */
if (value.data[j] <= 0x20 || value.data[j] == 0x7F
|| value.data[j] == ':') {
return NGX_ERROR;
}
}
return NGX_OK;
}