This commit is contained in:
Demi Marie Obenour 2025-05-30 00:17:04 +10:00 committed by GitHub
commit d5f7200727
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 252 additions and 3 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

@ -221,6 +221,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;
}
@ -306,6 +314,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;
@ -785,6 +801,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

@ -206,6 +206,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);
@ -416,7 +418,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 },
@ -1259,7 +1261,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;
@ -1409,6 +1411,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;
}
@ -1426,6 +1437,7 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
}
cl->buf = b;
cl->next = NULL;
/* the request line */
@ -1463,6 +1475,12 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
}
u->uri.len = b->last - u->uri.data;
if (ngx_http_valid_header_value(u->uri) != NGX_OK) {
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,
@ -1484,6 +1502,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);
@ -1508,14 +1527,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;
}
@ -4166,6 +4201,25 @@ 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;
ngx_str_t unstripped_value;
value = cf->args->elts;
unstripped_value = value[2];
if (ngx_conf_check_field_name_and_strip_value(cf, value, value + 1,
value + 2) != NGX_OK) {
value[2] = unstripped_value;
return NGX_CONF_ERROR;
}
rc = ngx_conf_set_keyval_slot(cf, cmd, conf);
value[2] = unstripped_value;
return rc;
}
static ngx_int_t
ngx_http_proxy_init_headers(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *conf,

View File

@ -2196,3 +2196,80 @@ 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)
{
u_char *out_cursor, *cursor, *end;
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;
}
if (ngx_http_conf_forbidden_header(*name)) {
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
"%V: cannot set %V as it would break HTTP/1.x "
"framing", (ngx_str_t *)cmd_name,
(ngx_str_t *)name);
return NGX_ERROR;
}
#define ISSPACE(c) ((c) == ' ' || (c) == '\t')
while (value->len > 0) {
if (!ISSPACE(value->data[0])) {
break;
}
value->len--;
value->data++;
}
while (value->len > 0) {
size_t last = value->len - 1;
if (!ISSPACE(value->data[last])) {
break;
}
value->len--;
}
end = value->len + value->data;
out_cursor = cursor = value->data;
for ( /* void */; cursor < end; ++cursor) {
if (cursor[0] == 0) {
goto fail;
}
if (cursor[0] != CR && cursor[0] != LF) {
*out_cursor++ = *cursor;
continue;
}
int iscr = cursor[0] == CR;
if (end - cursor >= iscr + 2 && cursor[iscr] == LF
&& ISSPACE(cursor[iscr + 1])) {
while (out_cursor > value->data && ISSPACE(out_cursor[-1])) {
out_cursor--;
}
cursor += iscr + 2;
while (end > cursor && ISSPACE(*cursor)) {
cursor++;
}
*out_cursor++ = ' ';
continue;
}
goto fail;
}
if (out_cursor < end) {
memset(out_cursor, '\0', (size_t)(end - out_cursor));
value->len -= (size_t)(end - 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

@ -119,6 +119,19 @@ 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 value is equal (case insensitive) to Transfer-Encoding or
* Content-Length. NGINX does not allow manually overriding these fields
* as it would make proper request framing impossible. */
ngx_int_t ngx_http_conf_forbidden_header(ngx_str_t key);
/** 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

@ -2413,3 +2413,80 @@ 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;
}
ngx_int_t
ngx_http_conf_forbidden_header(ngx_str_t key)
{
switch (key.len) {
case sizeof("Transfer-Encoding") - 1:
if (ngx_strncasecmp(key.data, (u_char *)"Transfer-Encoding",
key.len) == 0) {
return NGX_ERROR;
}
return NGX_OK;
case sizeof("Content-Length") - 1:
if (ngx_strncasecmp(key.data, (u_char *)"Content-Length",
key.len) == 0) {
return NGX_ERROR;
}
return NGX_OK;
default:
return NGX_OK;
}
}