From ce181590c8a78c43daa2144782a9804f17d887c4 Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Mon, 10 Nov 2025 20:47:32 +0400 Subject: [PATCH] HTTP/3: fixed misaligned access with Huffman encoding. The function expects to operate on aligned memory allocation. The fix is store encoded output in an aligned buffer passed via HTTP/3 encoder functions, similar to ngx_http_v2_filter_module.c. Tested with UndefinedBehaviorSanitizer. --- src/http/v3/ngx_http_v3_encode.c | 112 +++++++++--------------- src/http/v3/ngx_http_v3_encode.h | 6 +- src/http/v3/ngx_http_v3_filter_module.c | 99 ++++++++++++++++----- src/http/v3/ngx_http_v3_request.c | 3 +- 4 files changed, 120 insertions(+), 100 deletions(-) diff --git a/src/http/v3/ngx_http_v3_encode.c b/src/http/v3/ngx_http_v3_encode.c index fb089c413..d468b84c8 100644 --- a/src/http/v3/ngx_http_v3_encode.c +++ b/src/http/v3/ngx_http_v3_encode.c @@ -135,10 +135,9 @@ ngx_http_v3_encode_field_ri(u_char *p, ngx_uint_t dynamic, ngx_uint_t index) uintptr_t ngx_http_v3_encode_field_lri(u_char *p, ngx_uint_t dynamic, ngx_uint_t index, - u_char *data, size_t len) + u_char *data, size_t len, u_char *tmp) { - size_t hlen; - u_char *p1, *p2; + size_t hlen; /* Literal Field Line With Name Reference */ @@ -151,28 +150,22 @@ ngx_http_v3_encode_field_lri(u_char *p, ngx_uint_t dynamic, ngx_uint_t index, *p = dynamic ? 0x40 : 0x50; p = (u_char *) ngx_http_v3_encode_prefix_int(p, index, 4); - p1 = p; - *p = 0; - p = (u_char *) ngx_http_v3_encode_prefix_int(p, len, 7); + if (data == NULL) { + *p = 0; + return ngx_http_v3_encode_prefix_int(p, len, 7); + } - if (data) { - p2 = p; - hlen = ngx_http_huff_encode(data, len, p, 0); + hlen = ngx_http_huff_encode(data, len, tmp, 0); - if (hlen) { - p = p1; - *p = 0x80; - p = (u_char *) ngx_http_v3_encode_prefix_int(p, hlen, 7); + if (hlen) { + *p = 0x80; + p = (u_char *) ngx_http_v3_encode_prefix_int(p, hlen, 7); + p = ngx_cpymem(p, tmp, hlen); - if (p != p2) { - ngx_memmove(p, p2, hlen); - } - - p += hlen; - - } else { - p = ngx_cpymem(p, data, len); - } + } else { + *p = 0; + p = (u_char *) ngx_http_v3_encode_prefix_int(p, len, 7); + p = ngx_cpymem(p, data, len); } return (uintptr_t) p; @@ -180,10 +173,10 @@ ngx_http_v3_encode_field_lri(u_char *p, ngx_uint_t dynamic, ngx_uint_t index, uintptr_t -ngx_http_v3_encode_field_l(u_char *p, ngx_str_t *name, ngx_str_t *value) +ngx_http_v3_encode_field_l(u_char *p, ngx_str_t *name, ngx_str_t *value, + u_char *tmp) { size_t hlen; - u_char *p1, *p2; /* Literal Field Line With Literal Name */ @@ -194,48 +187,30 @@ ngx_http_v3_encode_field_l(u_char *p, ngx_str_t *name, ngx_str_t *value) + value->len; } - p1 = p; - *p = 0x20; - p = (u_char *) ngx_http_v3_encode_prefix_int(p, name->len, 3); - - p2 = p; - hlen = ngx_http_huff_encode(name->data, name->len, p, 1); + hlen = ngx_http_huff_encode(name->data, name->len, tmp, 1); if (hlen) { - p = p1; *p = 0x28; p = (u_char *) ngx_http_v3_encode_prefix_int(p, hlen, 3); - - if (p != p2) { - ngx_memmove(p, p2, hlen); - } - - p += hlen; + p = ngx_cpymem(p, tmp, hlen); } else { + *p = 0x20; + p = (u_char *) ngx_http_v3_encode_prefix_int(p, name->len, 3); ngx_strlow(p, name->data, name->len); p += name->len; } - p1 = p; - *p = 0; - p = (u_char *) ngx_http_v3_encode_prefix_int(p, value->len, 7); - - p2 = p; - hlen = ngx_http_huff_encode(value->data, value->len, p, 0); + hlen = ngx_http_huff_encode(value->data, value->len, tmp, 0); if (hlen) { - p = p1; *p = 0x80; p = (u_char *) ngx_http_v3_encode_prefix_int(p, hlen, 7); - - if (p != p2) { - ngx_memmove(p, p2, hlen); - } - - p += hlen; + p = ngx_cpymem(p, tmp, hlen); } else { + *p = 0; + p = (u_char *) ngx_http_v3_encode_prefix_int(p, value->len, 7); p = ngx_cpymem(p, value->data, value->len); } @@ -260,10 +235,9 @@ ngx_http_v3_encode_field_pbi(u_char *p, ngx_uint_t index) uintptr_t ngx_http_v3_encode_field_lpbi(u_char *p, ngx_uint_t index, u_char *data, - size_t len) + size_t len, u_char *tmp) { - size_t hlen; - u_char *p1, *p2; + size_t hlen; /* Literal Field Line With Post-Base Name Reference */ @@ -276,28 +250,22 @@ ngx_http_v3_encode_field_lpbi(u_char *p, ngx_uint_t index, u_char *data, *p = 0; p = (u_char *) ngx_http_v3_encode_prefix_int(p, index, 3); - p1 = p; - *p = 0; - p = (u_char *) ngx_http_v3_encode_prefix_int(p, len, 7); + if (data == NULL) { + *p = 0; + return ngx_http_v3_encode_prefix_int(p, len, 7); + } - if (data) { - p2 = p; - hlen = ngx_http_huff_encode(data, len, p, 0); + hlen = ngx_http_huff_encode(data, len, tmp, 0); - if (hlen) { - p = p1; - *p = 0x80; - p = (u_char *) ngx_http_v3_encode_prefix_int(p, hlen, 7); + if (hlen) { + *p = 0x80; + p = (u_char *) ngx_http_v3_encode_prefix_int(p, hlen, 7); + p = ngx_cpymem(p, tmp, hlen); - if (p != p2) { - ngx_memmove(p, p2, hlen); - } - - p += hlen; - - } else { - p = ngx_cpymem(p, data, len); - } + } else { + *p = 0; + p = (u_char *) ngx_http_v3_encode_prefix_int(p, len, 7); + p = ngx_cpymem(p, data, len); } return (uintptr_t) p; diff --git a/src/http/v3/ngx_http_v3_encode.h b/src/http/v3/ngx_http_v3_encode.h index fca376da5..6ba5dde35 100644 --- a/src/http/v3/ngx_http_v3_encode.h +++ b/src/http/v3/ngx_http_v3_encode.h @@ -23,12 +23,12 @@ uintptr_t ngx_http_v3_encode_field_section_prefix(u_char *p, uintptr_t ngx_http_v3_encode_field_ri(u_char *p, ngx_uint_t dynamic, ngx_uint_t index); uintptr_t ngx_http_v3_encode_field_lri(u_char *p, ngx_uint_t dynamic, - ngx_uint_t index, u_char *data, size_t len); + ngx_uint_t index, u_char *data, size_t len, u_char *tmp); uintptr_t ngx_http_v3_encode_field_l(u_char *p, ngx_str_t *name, - ngx_str_t *value); + ngx_str_t *value, u_char *tmp); uintptr_t ngx_http_v3_encode_field_pbi(u_char *p, ngx_uint_t index); uintptr_t ngx_http_v3_encode_field_lpbi(u_char *p, ngx_uint_t index, - u_char *data, size_t len); + u_char *data, size_t len, u_char *tmp); #endif /* _NGX_HTTP_V3_ENCODE_H_INCLUDED_ */ diff --git a/src/http/v3/ngx_http_v3_filter_module.c b/src/http/v3/ngx_http_v3_filter_module.c index e3f15368f..d0d34e503 100644 --- a/src/http/v3/ngx_http_v3_filter_module.c +++ b/src/http/v3/ngx_http_v3_filter_module.c @@ -84,8 +84,8 @@ static ngx_http_output_body_filter_pt ngx_http_next_body_filter; static ngx_int_t ngx_http_v3_header_filter(ngx_http_request_t *r) { - u_char *p; - size_t len, n; + u_char *p, *tmp; + size_t len, tmp_len, n; ngx_buf_t *b; ngx_str_t host, location; ngx_uint_t i, port; @@ -156,7 +156,7 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) } else { len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_STATUS_200, - NULL, 3); + NULL, 3, NULL); } clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); @@ -174,12 +174,13 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_SERVER, - NULL, n); + NULL, n, NULL); } if (r->headers_out.date == NULL) { len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_DATE, - NULL, ngx_cached_http_time.len); + NULL, ngx_cached_http_time.len, + NULL); } if (r->headers_out.content_type.len) { @@ -193,14 +194,14 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_CONTENT_TYPE_TEXT_PLAIN, - NULL, n); + NULL, n, NULL); } if (r->headers_out.content_length == NULL) { if (r->headers_out.content_length_n > 0) { len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_CONTENT_LENGTH_ZERO, - NULL, NGX_OFF_T_LEN); + NULL, NGX_OFF_T_LEN, NULL); } else if (r->headers_out.content_length_n == 0) { len += ngx_http_v3_encode_field_ri(NULL, 0, @@ -213,7 +214,8 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) { len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_LAST_MODIFIED, NULL, - sizeof("Mon, 28 Sep 1970 06:00:00 GMT") - 1); + sizeof("Mon, 28 Sep 1970 06:00:00 GMT") - 1, + NULL); } if (r->headers_out.location && r->headers_out.location->value.len) { @@ -279,9 +281,12 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) len += ngx_http_v3_encode_field_lri(NULL, 0, NGX_HTTP_V3_HEADER_LOCATION, NULL, - r->headers_out.location->value.len); + r->headers_out.location->value.len, + NULL); } + tmp_len = len; + #if (NGX_HTTP_GZIP) if (r->gzip_vary) { if (clcf->gzip_vary) { @@ -314,7 +319,20 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) } len += ngx_http_v3_encode_field_l(NULL, &header[i].key, - &header[i].value); + &header[i].value, NULL); + + if (header[i].key.len > tmp_len) { + tmp_len = header[i].key.len; + } + + if (header[i].value.len > tmp_len) { + tmp_len = header[i].value.len; + } + } + + tmp = ngx_palloc(r->pool, tmp_len); + if (tmp == NULL) { + return NGX_ERROR; } ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 header len:%uz", len); @@ -338,7 +356,7 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) } else { b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_STATUS_200, - NULL, 3); + NULL, 3, tmp); b->last = ngx_sprintf(b->last, "%03ui", r->headers_out.status); } @@ -361,7 +379,7 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_SERVER, - p, n); + p, n, tmp); } if (r->headers_out.date == NULL) { @@ -372,7 +390,8 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_DATE, ngx_cached_http_time.data, - ngx_cached_http_time.len); + ngx_cached_http_time.len, + tmp); } if (r->headers_out.content_type.len) { @@ -408,7 +427,8 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_CONTENT_TYPE_TEXT_PLAIN, r->headers_out.content_type.data, - r->headers_out.content_type.len); + r->headers_out.content_type.len, + tmp); } if (r->headers_out.content_length == NULL @@ -424,7 +444,7 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_CONTENT_LENGTH_ZERO, - NULL, n); + NULL, n, tmp); b->last = ngx_sprintf(b->last, "%O", r->headers_out.content_length_n); @@ -452,7 +472,7 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_LAST_MODIFIED, - p, n); + p, n, tmp); } if (r->headers_out.location && r->headers_out.location->value.len) { @@ -463,7 +483,8 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_lri(b->last, 0, NGX_HTTP_V3_HEADER_LOCATION, r->headers_out.location->value.data, - r->headers_out.location->value.len); + r->headers_out.location->value.len, + tmp); } #if (NGX_HTTP_GZIP) @@ -501,7 +522,7 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_l(b->last, &header[i].key, - &header[i].value); + &header[i].value, tmp); } if (r->header_only) { @@ -594,7 +615,8 @@ ngx_http_v3_header_filter(ngx_http_request_t *r) static ngx_int_t ngx_http_v3_early_hints_filter(ngx_http_request_t *r) { - size_t len, n; + u_char *tmp; + size_t len, n, tmp_len; ngx_buf_t *b; ngx_uint_t i; ngx_chain_t *out, *hl, *cl; @@ -611,6 +633,7 @@ ngx_http_v3_early_hints_filter(ngx_http_request_t *r) } len = 0; + tmp_len = 0; part = &r->headers_out.headers.part; header = part->elts; @@ -632,7 +655,15 @@ ngx_http_v3_early_hints_filter(ngx_http_request_t *r) } len += ngx_http_v3_encode_field_l(NULL, &header[i].key, - &header[i].value); + &header[i].value, NULL); + + if (header[i].key.len > tmp_len) { + tmp_len = header[i].key.len; + } + + if (header[i].value.len > tmp_len) { + tmp_len = header[i].value.len; + } } if (len == 0) { @@ -646,6 +677,11 @@ ngx_http_v3_early_hints_filter(ngx_http_request_t *r) ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "http3 header len:%uz", len); + tmp = ngx_palloc(r->pool, tmp_len); + if (tmp == NULL) { + return NGX_ERROR; + } + b = ngx_create_temp_buf(r->pool, len); if (b == NULL) { return NGX_ERROR; @@ -686,7 +722,7 @@ ngx_http_v3_early_hints_filter(ngx_http_request_t *r) b->last = (u_char *) ngx_http_v3_encode_field_l(b->last, &header[i].key, - &header[i].value); + &header[i].value, tmp); } b->flush = 1; @@ -856,7 +892,8 @@ static ngx_chain_t * ngx_http_v3_create_trailers(ngx_http_request_t *r, ngx_http_v3_filter_ctx_t *ctx) { - size_t len, n; + u_char *tmp; + size_t len, n, tmp_len; u_char *p; ngx_buf_t *b; ngx_uint_t i; @@ -868,6 +905,7 @@ ngx_http_v3_create_trailers(ngx_http_request_t *r, h3c = ngx_http_v3_get_session(r->connection); len = 0; + tmp_len = 0; part = &r->headers_out.trailers.part; header = part->elts; @@ -889,7 +927,15 @@ ngx_http_v3_create_trailers(ngx_http_request_t *r, } len += ngx_http_v3_encode_field_l(NULL, &header[i].key, - &header[i].value); + &header[i].value, NULL); + + if (header[i].key.len > tmp_len) { + tmp_len = header[i].key.len; + } + + if (header[i].value.len > tmp_len) { + tmp_len = header[i].value.len; + } } cl = ngx_chain_get_free_buf(r->pool, &ctx->free); @@ -911,6 +957,11 @@ ngx_http_v3_create_trailers(ngx_http_request_t *r, b->temporary = 1; + tmp = ngx_palloc(r->pool, tmp_len); + if (tmp == NULL) { + return NULL; + } + len += ngx_http_v3_encode_field_section_prefix(NULL, 0, 0, 0); b->pos = ngx_palloc(r->pool, len); @@ -946,7 +997,7 @@ ngx_http_v3_create_trailers(ngx_http_request_t *r, b->last = (u_char *) ngx_http_v3_encode_field_l(b->last, &header[i].key, - &header[i].value); + &header[i].value, tmp); } n = b->last - b->pos; diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index 77c55bfee..92a5f97d6 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -584,7 +584,8 @@ ngx_http_v3_process_request(ngx_event_t *rev) h3c->payload_bytes += ngx_http_v3_encode_field_l(NULL, &st->field_rep.field.name, - &st->field_rep.field.value); + &st->field_rep.field.value, + NULL); if (ngx_http_v3_process_header(r, &st->field_rep.field.name, &st->field_rep.field.value)