From: Maxim Dounin Date: Fri, 19 Jun 2026 19:55:38 +0000 (+0300) Subject: Access log: buffer overrun protection. X-Git-Tag: release-1.31.3~5 X-Git-Url: http://git.kaiwu.me/http/doc/$%7BGITURL%7D/static/gitweb.js?a=commitdiff_plain;h=4d32a2703c79f92b7a99ce3547759cc599d6f83e;p=nginx.git Access log: buffer overrun protection. Similarly to generic script operations, access log script copy operations now check if there is enough room in the buffer. Signed-off-by: Roman Arutyunyan Origin: --- diff --git a/src/http/modules/ngx_http_log_module.c b/src/http/modules/ngx_http_log_module.c index 57fb1d377..91589747e 100644 --- a/src/http/modules/ngx_http_log_module.c +++ b/src/http/modules/ngx_http_log_module.c @@ -17,7 +17,7 @@ typedef struct ngx_http_log_op_s ngx_http_log_op_t; typedef u_char *(*ngx_http_log_op_run_pt) (ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); typedef size_t (*ngx_http_log_op_getlen_pt) (ngx_http_request_t *r, uintptr_t data); @@ -112,39 +112,42 @@ static void ngx_http_log_flush(ngx_open_file_t *file, ngx_log_t *log); static void ngx_http_log_flush_handler(ngx_event_t *ev); static u_char *ngx_http_log_pipe(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_time(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_iso8601(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_msec(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_request_time(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_status(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_bytes_sent(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); -static u_char *ngx_http_log_body_bytes_sent(ngx_http_request_t *r, - u_char *buf, ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); +static u_char *ngx_http_log_body_bytes_sent(ngx_http_request_t *r, u_char *buf, + u_char *end, ngx_http_log_op_t *op); static u_char *ngx_http_log_request_length(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static ngx_int_t ngx_http_log_variable_compile(ngx_conf_t *cf, ngx_http_log_op_t *op, ngx_str_t *value, ngx_uint_t escape); static size_t ngx_http_log_variable_getlen(ngx_http_request_t *r, uintptr_t data); static u_char *ngx_http_log_variable(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static uintptr_t ngx_http_log_escape(u_char *dst, u_char *src, size_t size); static size_t ngx_http_log_json_variable_getlen(ngx_http_request_t *r, uintptr_t data); static u_char *ngx_http_log_json_variable(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op); + u_char *end, ngx_http_log_op_t *op); static size_t ngx_http_log_unescaped_variable_getlen(ngx_http_request_t *r, uintptr_t data); static u_char *ngx_http_log_unescaped_variable(ngx_http_request_t *r, - u_char *buf, ngx_http_log_op_t *op); + u_char *buf, u_char *end, ngx_http_log_op_t *op); + +static ngx_int_t ngx_http_log_check_length(ngx_http_request_t *r, + u_char *buf, u_char *end, size_t len); static void *ngx_http_log_create_main_conf(ngx_conf_t *cf); @@ -253,7 +256,7 @@ static ngx_http_log_var_t ngx_http_log_vars[] = { static ngx_int_t ngx_http_log_handler(ngx_http_request_t *r) { - u_char *line, *p; + u_char *line, *p, *end; size_t len, size; ssize_t n; ngx_str_t val; @@ -309,6 +312,8 @@ ngx_http_log_handler(ngx_http_request_t *r) } } + len += NGX_LINEFEED_SIZE; + if (log[l].syslog_peer) { /* length of syslog's PRI and HEADER message parts */ @@ -319,8 +324,6 @@ ngx_http_log_handler(ngx_http_request_t *r) goto alloc_line; } - len += NGX_LINEFEED_SIZE; - buffer = log[l].file ? log[l].file->data : NULL; if (buffer) { @@ -336,13 +339,18 @@ ngx_http_log_handler(ngx_http_request_t *r) if (len <= (size_t) (buffer->last - buffer->pos)) { p = buffer->pos; + end = p + len - NGX_LINEFEED_SIZE; if (buffer->event && p == buffer->start) { ngx_add_timer(buffer->event, buffer->flush); } - for (i = 0; i < log[l].format->ops->nelts; i++) { - p = op[i].run(r, p, &op[i]); + for (i = 0; i < log[l].format->ops->nelts && p; i++) { + p = op[i].run(r, p, end, &op[i]); + } + + if (p == NULL) { + return NGX_ERROR; } ngx_linefeed(p); @@ -365,13 +373,18 @@ ngx_http_log_handler(ngx_http_request_t *r) } p = line; + end = line + len - NGX_LINEFEED_SIZE; if (log[l].syslog_peer) { p = ngx_syslog_add_header(log[l].syslog_peer, line); } - for (i = 0; i < log[l].format->ops->nelts; i++) { - p = op[i].run(r, p, &op[i]); + for (i = 0; i < log[l].format->ops->nelts && p; i++) { + p = op[i].run(r, p, end, &op[i]); + } + + if (p == NULL) { + return NGX_ERROR; } if (log[l].syslog_peer) { @@ -770,7 +783,7 @@ ngx_http_log_flush_handler(ngx_event_t *ev) static u_char * -ngx_http_log_copy_short(ngx_http_request_t *r, u_char *buf, +ngx_http_log_copy_short(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { size_t len; @@ -779,6 +792,10 @@ ngx_http_log_copy_short(ngx_http_request_t *r, u_char *buf, len = op->len; data = op->data; + if (ngx_http_log_check_length(r, buf, end, len) != NGX_OK) { + return NULL; + } + while (len--) { *buf++ = (u_char) (data & 0xff); data >>= 8; @@ -789,16 +806,25 @@ ngx_http_log_copy_short(ngx_http_request_t *r, u_char *buf, static u_char * -ngx_http_log_copy_long(ngx_http_request_t *r, u_char *buf, +ngx_http_log_copy_long(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { + if (ngx_http_log_check_length(r, buf, end, op->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, (u_char *) op->data, op->len); } static u_char * -ngx_http_log_pipe(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) +ngx_http_log_pipe(ngx_http_request_t *r, u_char *buf, u_char *end, + ngx_http_log_op_t *op) { + if (ngx_http_log_check_length(r, buf, end, 1) != NGX_OK) { + return NULL; + } + if (r->pipeline) { *buf = 'p'; } else { @@ -810,24 +836,43 @@ ngx_http_log_pipe(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) static u_char * -ngx_http_log_time(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) +ngx_http_log_time(ngx_http_request_t *r, u_char *buf, u_char *end, + ngx_http_log_op_t *op) { + if (ngx_http_log_check_length(r, buf, end, ngx_cached_http_log_time.len) + != NGX_OK) + { + return NULL; + } + return ngx_cpymem(buf, ngx_cached_http_log_time.data, ngx_cached_http_log_time.len); } static u_char * -ngx_http_log_iso8601(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) +ngx_http_log_iso8601(ngx_http_request_t *r, u_char *buf, u_char *end, + ngx_http_log_op_t *op) { + if (ngx_http_log_check_length(r, buf, end, ngx_cached_http_log_iso8601.len) + != NGX_OK) + { + return NULL; + } + return ngx_cpymem(buf, ngx_cached_http_log_iso8601.data, ngx_cached_http_log_iso8601.len); } static u_char * -ngx_http_log_msec(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) +ngx_http_log_msec(ngx_http_request_t *r, u_char *buf, u_char *end, + ngx_http_log_op_t *op) { ngx_time_t *tp; + if (ngx_http_log_check_length(r, buf, end, NGX_TIME_T_LEN + 4) != NGX_OK) { + return NULL; + } + tp = ngx_timeofday(); return ngx_sprintf(buf, "%T.%03M", tp->sec, tp->msec); @@ -835,12 +880,16 @@ ngx_http_log_msec(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) static u_char * -ngx_http_log_request_time(ngx_http_request_t *r, u_char *buf, +ngx_http_log_request_time(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { ngx_time_t *tp; ngx_msec_int_t ms; + if (ngx_http_log_check_length(r, buf, end, NGX_TIME_T_LEN + 4) != NGX_OK) { + return NULL; + } + tp = ngx_timeofday(); ms = (ngx_msec_int_t) @@ -852,10 +901,15 @@ ngx_http_log_request_time(ngx_http_request_t *r, u_char *buf, static u_char * -ngx_http_log_status(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) +ngx_http_log_status(ngx_http_request_t *r, u_char *buf, u_char *end, + ngx_http_log_op_t *op) { ngx_uint_t status; + if (ngx_http_log_check_length(r, buf, end, NGX_INT_T_LEN) != NGX_OK) { + return NULL; + } + if (r->err_status) { status = r->err_status; @@ -874,9 +928,13 @@ ngx_http_log_status(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) static u_char * -ngx_http_log_bytes_sent(ngx_http_request_t *r, u_char *buf, +ngx_http_log_bytes_sent(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { + if (ngx_http_log_check_length(r, buf, end, NGX_OFF_T_LEN) != NGX_OK) { + return NULL; + } + return ngx_sprintf(buf, "%O", r->connection->sent); } @@ -887,11 +945,15 @@ ngx_http_log_bytes_sent(ngx_http_request_t *r, u_char *buf, */ static u_char * -ngx_http_log_body_bytes_sent(ngx_http_request_t *r, u_char *buf, +ngx_http_log_body_bytes_sent(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { off_t length; + if (ngx_http_log_check_length(r, buf, end, NGX_OFF_T_LEN) != NGX_OK) { + return NULL; + } + length = r->connection->sent - r->header_size; if (length > 0) { @@ -905,9 +967,13 @@ ngx_http_log_body_bytes_sent(ngx_http_request_t *r, u_char *buf, static u_char * -ngx_http_log_request_length(ngx_http_request_t *r, u_char *buf, +ngx_http_log_request_length(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { + if (ngx_http_log_check_length(r, buf, end, NGX_OFF_T_LEN) != NGX_OK) { + return NULL; + } + return ngx_sprintf(buf, "%O", r->request_length); } @@ -968,21 +1034,39 @@ ngx_http_log_variable_getlen(ngx_http_request_t *r, uintptr_t data) static u_char * -ngx_http_log_variable(ngx_http_request_t *r, u_char *buf, ngx_http_log_op_t *op) +ngx_http_log_variable(ngx_http_request_t *r, u_char *buf, u_char *end, + ngx_http_log_op_t *op) { + uintptr_t len; ngx_http_variable_value_t *value; value = ngx_http_get_indexed_variable(r, op->data); if (value == NULL || value->not_found) { + if (ngx_http_log_check_length(r, buf, end, 1) != NGX_OK) { + return NULL; + } + *buf = '-'; return buf + 1; } if (value->escape == 0) { + if (ngx_http_log_check_length(r, buf, end, value->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, value->data, value->len); } else { + len = ngx_http_log_escape(NULL, value->data, value->len); + + if (ngx_http_log_check_length(r, buf, end, value->len + len * 3) + != NGX_OK) + { + return NULL; + } + return (u_char *) ngx_http_log_escape(buf, value->data, value->len); } } @@ -1069,9 +1153,10 @@ ngx_http_log_json_variable_getlen(ngx_http_request_t *r, uintptr_t data) static u_char * -ngx_http_log_json_variable(ngx_http_request_t *r, u_char *buf, +ngx_http_log_json_variable(ngx_http_request_t *r, u_char *buf, u_char *end, ngx_http_log_op_t *op) { + uintptr_t len; ngx_http_variable_value_t *value; value = ngx_http_get_indexed_variable(r, op->data); @@ -1081,9 +1166,21 @@ ngx_http_log_json_variable(ngx_http_request_t *r, u_char *buf, } if (value->escape == 0) { + if (ngx_http_log_check_length(r, buf, end, value->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, value->data, value->len); } else { + len = ngx_escape_json(NULL, value->data, value->len); + + if (ngx_http_log_check_length(r, buf, end, value->len + len) + != NGX_OK) + { + return NULL; + } + return (u_char *) ngx_escape_json(buf, value->data, value->len); } } @@ -1108,7 +1205,7 @@ ngx_http_log_unescaped_variable_getlen(ngx_http_request_t *r, uintptr_t data) static u_char * ngx_http_log_unescaped_variable(ngx_http_request_t *r, u_char *buf, - ngx_http_log_op_t *op) + u_char *end, ngx_http_log_op_t *op) { ngx_http_variable_value_t *value; @@ -1118,10 +1215,28 @@ ngx_http_log_unescaped_variable(ngx_http_request_t *r, u_char *buf, return buf; } + if (ngx_http_log_check_length(r, buf, end, value->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, value->data, value->len); } +static ngx_int_t +ngx_http_log_check_length(ngx_http_request_t *r, u_char *buf, u_char *end, + size_t len) +{ + if (end - buf < (ssize_t) len) { + ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0, + "no buffer space in log script copy"); + return NGX_ERROR; + } + + return NGX_OK; +} + + static void * ngx_http_log_create_main_conf(ngx_conf_t *cf) { diff --git a/src/stream/ngx_stream_log_module.c b/src/stream/ngx_stream_log_module.c index 0ff7f4244..774bc7e93 100644 --- a/src/stream/ngx_stream_log_module.c +++ b/src/stream/ngx_stream_log_module.c @@ -17,7 +17,7 @@ typedef struct ngx_stream_log_op_s ngx_stream_log_op_t; typedef u_char *(*ngx_stream_log_op_run_pt) (ngx_stream_session_t *s, - u_char *buf, ngx_stream_log_op_t *op); + u_char *buf, u_char *end, ngx_stream_log_op_t *op); typedef size_t (*ngx_stream_log_op_getlen_pt) (ngx_stream_session_t *s, uintptr_t data); @@ -115,16 +115,19 @@ static ngx_int_t ngx_stream_log_variable_compile(ngx_conf_t *cf, static size_t ngx_stream_log_variable_getlen(ngx_stream_session_t *s, uintptr_t data); static u_char *ngx_stream_log_variable(ngx_stream_session_t *s, u_char *buf, - ngx_stream_log_op_t *op); + u_char *end, ngx_stream_log_op_t *op); static uintptr_t ngx_stream_log_escape(u_char *dst, u_char *src, size_t size); static size_t ngx_stream_log_json_variable_getlen(ngx_stream_session_t *s, uintptr_t data); static u_char *ngx_stream_log_json_variable(ngx_stream_session_t *s, - u_char *buf, ngx_stream_log_op_t *op); + u_char *buf, u_char *end, ngx_stream_log_op_t *op); static size_t ngx_stream_log_unescaped_variable_getlen(ngx_stream_session_t *s, uintptr_t data); static u_char *ngx_stream_log_unescaped_variable(ngx_stream_session_t *s, - u_char *buf, ngx_stream_log_op_t *op); + u_char *buf, u_char *end, ngx_stream_log_op_t *op); + +static ngx_int_t ngx_stream_log_check_length(ngx_stream_session_t *s, + u_char *buf, u_char *end, size_t len); static void *ngx_stream_log_create_main_conf(ngx_conf_t *cf); @@ -200,7 +203,7 @@ ngx_module_t ngx_stream_log_module = { static ngx_int_t ngx_stream_log_handler(ngx_stream_session_t *s) { - u_char *line, *p; + u_char *line, *p, *end; size_t len, size; ssize_t n; ngx_str_t val; @@ -257,6 +260,8 @@ ngx_stream_log_handler(ngx_stream_session_t *s) } } + len += NGX_LINEFEED_SIZE; + if (log[l].syslog_peer) { /* length of syslog's PRI and HEADER message parts */ @@ -267,8 +272,6 @@ ngx_stream_log_handler(ngx_stream_session_t *s) goto alloc_line; } - len += NGX_LINEFEED_SIZE; - buffer = log[l].file ? log[l].file->data : NULL; if (buffer) { @@ -284,13 +287,18 @@ ngx_stream_log_handler(ngx_stream_session_t *s) if (len <= (size_t) (buffer->last - buffer->pos)) { p = buffer->pos; + end = p + len - NGX_LINEFEED_SIZE; if (buffer->event && p == buffer->start) { ngx_add_timer(buffer->event, buffer->flush); } - for (i = 0; i < log[l].format->ops->nelts; i++) { - p = op[i].run(s, p, &op[i]); + for (i = 0; i < log[l].format->ops->nelts && p; i++) { + p = op[i].run(s, p, end, &op[i]); + } + + if (p == NULL) { + return NGX_ERROR; } ngx_linefeed(p); @@ -313,13 +321,18 @@ ngx_stream_log_handler(ngx_stream_session_t *s) } p = line; + end = p + len - NGX_LINEFEED_SIZE; if (log[l].syslog_peer) { p = ngx_syslog_add_header(log[l].syslog_peer, line); } - for (i = 0; i < log[l].format->ops->nelts; i++) { - p = op[i].run(s, p, &op[i]); + for (i = 0; i < log[l].format->ops->nelts && p; i++) { + p = op[i].run(s, p, end, &op[i]); + } + + if (p == NULL) { + return NGX_ERROR; } if (log[l].syslog_peer) { @@ -663,7 +676,7 @@ ngx_stream_log_flush_handler(ngx_event_t *ev) static u_char * -ngx_stream_log_copy_short(ngx_stream_session_t *s, u_char *buf, +ngx_stream_log_copy_short(ngx_stream_session_t *s, u_char *buf, u_char *end, ngx_stream_log_op_t *op) { size_t len; @@ -672,6 +685,10 @@ ngx_stream_log_copy_short(ngx_stream_session_t *s, u_char *buf, len = op->len; data = op->data; + if (ngx_stream_log_check_length(s, buf, end, len) != NGX_OK) { + return NULL; + } + while (len--) { *buf++ = (u_char) (data & 0xff); data >>= 8; @@ -682,9 +699,13 @@ ngx_stream_log_copy_short(ngx_stream_session_t *s, u_char *buf, static u_char * -ngx_stream_log_copy_long(ngx_stream_session_t *s, u_char *buf, +ngx_stream_log_copy_long(ngx_stream_session_t *s, u_char *buf, u_char *end, ngx_stream_log_op_t *op) { + if (ngx_stream_log_check_length(s, buf, end, op->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, (u_char *) op->data, op->len); } @@ -745,22 +766,39 @@ ngx_stream_log_variable_getlen(ngx_stream_session_t *s, uintptr_t data) static u_char * -ngx_stream_log_variable(ngx_stream_session_t *s, u_char *buf, +ngx_stream_log_variable(ngx_stream_session_t *s, u_char *buf, u_char *end, ngx_stream_log_op_t *op) { + uintptr_t len; ngx_stream_variable_value_t *value; value = ngx_stream_get_indexed_variable(s, op->data); if (value == NULL || value->not_found) { + if (ngx_stream_log_check_length(s, buf, end, 1) != NGX_OK) { + return NULL; + } + *buf = '-'; return buf + 1; } if (value->escape == 0) { + if (ngx_stream_log_check_length(s, buf, end, value->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, value->data, value->len); } else { + len = ngx_stream_log_escape(NULL, value->data, value->len); + + if (ngx_stream_log_check_length(s, buf, end, value->len + len * 3) + != NGX_OK) + { + return NULL; + } + return (u_char *) ngx_stream_log_escape(buf, value->data, value->len); } } @@ -847,9 +885,10 @@ ngx_stream_log_json_variable_getlen(ngx_stream_session_t *s, uintptr_t data) static u_char * -ngx_stream_log_json_variable(ngx_stream_session_t *s, u_char *buf, +ngx_stream_log_json_variable(ngx_stream_session_t *s, u_char *buf, u_char *end, ngx_stream_log_op_t *op) { + uintptr_t len; ngx_stream_variable_value_t *value; value = ngx_stream_get_indexed_variable(s, op->data); @@ -859,9 +898,21 @@ ngx_stream_log_json_variable(ngx_stream_session_t *s, u_char *buf, } if (value->escape == 0) { + if (ngx_stream_log_check_length(s, buf, end, value->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, value->data, value->len); } else { + len = ngx_escape_json(NULL, value->data, value->len); + + if (ngx_stream_log_check_length(s, buf, end, value->len + len) + != NGX_OK) + { + return NULL; + } + return (u_char *) ngx_escape_json(buf, value->data, value->len); } } @@ -887,7 +938,7 @@ ngx_stream_log_unescaped_variable_getlen(ngx_stream_session_t *s, static u_char * ngx_stream_log_unescaped_variable(ngx_stream_session_t *s, u_char *buf, - ngx_stream_log_op_t *op) + u_char *end, ngx_stream_log_op_t *op) { ngx_stream_variable_value_t *value; @@ -897,10 +948,28 @@ ngx_stream_log_unescaped_variable(ngx_stream_session_t *s, u_char *buf, return buf; } + if (ngx_stream_log_check_length(s, buf, end, value->len) != NGX_OK) { + return NULL; + } + return ngx_cpymem(buf, value->data, value->len); } +static ngx_int_t +ngx_stream_log_check_length(ngx_stream_session_t *s, u_char *buf, u_char *end, + size_t len) +{ + if (end - buf < (ssize_t) len) { + ngx_log_error(NGX_LOG_ALERT, s->connection->log, 0, + "no buffer space in log script copy"); + return NGX_ERROR; + } + + return NGX_OK; +} + + static void * ngx_stream_log_create_main_conf(ngx_conf_t *cf) {