From 41ef44497865592bc904bfdf144309749e3f6c68 Mon Sep 17 00:00:00 2001 From: PerryLink <255665900+PerryLink@users.noreply.github.com> Date: Sun, 18 Oct 2026 13:19:51 +0000 Subject: [PATCH 1/4] fix out-of-bounds write when the conf server returns more than 20 headers `state.headers' is a fixed table of NGX_MAX_HEADERS entries indexed with `state.num_headers - 1', and `ngx_http_header_field_cb()' incremented `num_headers' without checking the bound. The 21st header of the conf-server reply was written past the end of the table, clobbering `num_headers'/`last_header' in `ngx_http_state'; the next value callback then indexed the table with the corrupted counter and the worker dereferenced a wild pointer. Stop recording once the table is full: add an IGNORED state so the field callback does not increment past NGX_MAX_HEADERS and the value callback skips the value of a dropped header. The first 20 headers, including X-Consul-Index and X-Etcd-Index, keep being recorded. --- src/ngx_http_upsync_module.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/ngx_http_upsync_module.c b/src/ngx_http_upsync_module.c index 8864cac..71f3238 100644 --- a/src/ngx_http_upsync_module.c +++ b/src/ngx_http_upsync_module.c @@ -149,7 +149,7 @@ typedef struct { ngx_uint_t num_headers; - enum { NONE=0, FIELD, VALUE } last_header; + enum { NONE=0, FIELD, VALUE, IGNORED } last_header; u_char http_body[NGX_PAGE_SIZE * NGX_PAGE_NUMBER]; } ngx_http_state; @@ -3432,7 +3432,22 @@ ngx_http_header_field_cb (http_parser *p, const char *buf, size_t len) return NGX_ERROR; } + if (state.last_header == IGNORED) { + return NGX_OK; + } + if (state.last_header != FIELD) { + + if (state.num_headers >= NGX_MAX_HEADERS) { + /* + * The fixed-size header table is full. Ignore this and every + * following header: indexing state.headers[] with num_headers + * here would write past the end of the array. + */ + state.last_header = IGNORED; + return NGX_OK; + } + state.num_headers++; } @@ -3456,6 +3471,10 @@ ngx_http_header_value_cb (http_parser *p, const char *buf, size_t len) return NGX_ERROR; } + if (state.last_header == IGNORED) { + return NGX_OK; + } + ngx_strlncat(state.headers[state.num_headers-1][1], sizeof(state.headers[state.num_headers-1][1]), buf, From 42b0850a5f5ca873c8c05d7eb78b8a17ce5f763f Mon Sep 17 00:00:00 2001 From: PerryLink <255665900+PerryLink@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:16:02 +0000 Subject: [PATCH 2/4] fix: warn once when the conf server response overflows the header table Dropping the surplus headers keeps the worker alive, but if the index header is among the dropped ones the upstream silently stops syncing and the only log line is \upstream index has not changed\, which points the wrong way. Log a warning the first time the table fills up in a worker. Suggested by @gfrankliu in review. --- src/ngx_http_upsync_module.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/ngx_http_upsync_module.c b/src/ngx_http_upsync_module.c index 71f3238..54eab4e 100644 --- a/src/ngx_http_upsync_module.c +++ b/src/ngx_http_upsync_module.c @@ -267,6 +267,9 @@ ngx_atomic_t *upsync_shared_created = &upsync_shared_created0; static http_parser *parser = NULL; static ngx_http_state state; +/* Warn once per worker when a conf-server reply overflows the header table. */ +static ngx_uint_t upsync_header_overflow_warned = 0; + static ngx_http_upsync_main_conf_t *upsync_ctx = NULL; static ngx_command_t ngx_http_upsync_commands[] = { @@ -3443,7 +3446,19 @@ ngx_http_header_field_cb (http_parser *p, const char *buf, size_t len) * The fixed-size header table is full. Ignore this and every * following header: indexing state.headers[] with num_headers * here would write past the end of the array. + * + * Dropping the surplus headers is silent by design (the response is + * still usable), but it is worth one warning per worker: if the + * index header is among the dropped ones the upstream simply stops + * syncing, and "upstream index has not changed" points the wrong way. */ + if (!upsync_header_overflow_warned) { + upsync_header_overflow_warned = 1; + ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0, + "upsync: conf server response has more than %d " + "headers, ignoring the rest", NGX_MAX_HEADERS); + } + state.last_header = IGNORED; return NGX_OK; } From e0a70175d96cfd8f5c8b9283ee3c61052b6a2cdd Mon Sep 17 00:00:00 2001 From: PerryLink <255665900+PerryLink@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:30:33 +0000 Subject: [PATCH 3/4] Log a warning when the header table overflows --- src/ngx_http_upsync_module.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/ngx_http_upsync_module.c b/src/ngx_http_upsync_module.c index 54eab4e..4053d54 100644 --- a/src/ngx_http_upsync_module.c +++ b/src/ngx_http_upsync_module.c @@ -267,9 +267,6 @@ ngx_atomic_t *upsync_shared_created = &upsync_shared_created0; static http_parser *parser = NULL; static ngx_http_state state; -/* Warn once per worker when a conf-server reply overflows the header table. */ -static ngx_uint_t upsync_header_overflow_warned = 0; - static ngx_http_upsync_main_conf_t *upsync_ctx = NULL; static ngx_command_t ngx_http_upsync_commands[] = { @@ -3447,17 +3444,15 @@ ngx_http_header_field_cb (http_parser *p, const char *buf, size_t len) * following header: indexing state.headers[] with num_headers * here would write past the end of the array. * - * Dropping the surplus headers is silent by design (the response is - * still usable), but it is worth one warning per worker: if the - * index header is among the dropped ones the upstream simply stops - * syncing, and "upstream index has not changed" points the wrong way. + * Logged once per reply: the IGNORED state above returns early + * from every later header, so this branch runs at most once. + * Silence here would be hard to diagnose: a dropped + * X-Consul-Index or X-Etcd-Index leaves the index at 0, which + * upsync_check_index() reports as "index has not changed". */ - if (!upsync_header_overflow_warned) { - upsync_header_overflow_warned = 1; - ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0, - "upsync: conf server response has more than %d " - "headers, ignoring the rest", NGX_MAX_HEADERS); - } + ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0, + "upsync: conf server response has more than %d " + "headers, ignoring the rest", NGX_MAX_HEADERS); state.last_header = IGNORED; return NGX_OK; From 38090e60a6595094b51e82d83a61a23aeca87ded Mon Sep 17 00:00:00 2001 From: PerryLink <255665900+PerryLink@users.noreply.github.com> Date: Mon, 19 Oct 2026 04:05:38 +0000 Subject: [PATCH 4/4] Log the header-table overflow at error level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nginx's default error_log level is error, so the WARN added in e0a7017 is invisible in a stock configuration — the same reason the "index has not changed" NOTICE is easy to miss. A dropped X-Consul-Index stops syncing completely, so the line belongs at NGX_LOG_ERR. --- src/ngx_http_upsync_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ngx_http_upsync_module.c b/src/ngx_http_upsync_module.c index 4053d54..c24d69c 100644 --- a/src/ngx_http_upsync_module.c +++ b/src/ngx_http_upsync_module.c @@ -3450,7 +3450,7 @@ ngx_http_header_field_cb (http_parser *p, const char *buf, size_t len) * X-Consul-Index or X-Etcd-Index leaves the index at 0, which * upsync_check_index() reports as "index has not changed". */ - ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0, + ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0, "upsync: conf server response has more than %d " "headers, ignoring the rest", NGX_MAX_HEADERS);