From b6ad5a6e30297fbbfc7cde42de800bd0637b4c67 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 03:30:07 -0500 Subject: [PATCH 01/14] Fix possible memory corruption when using upstream requests --- src/subscribers/websocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/subscribers/websocket.c b/src/subscribers/websocket.c index 3e395f48..64086b91 100644 --- a/src/subscribers/websocket.c +++ b/src/subscribers/websocket.c @@ -671,10 +671,10 @@ static ngx_int_t websocket_publish(full_subscriber_t *fsub, ngx_buf_t *buf, int //move the msg pool d->pool = fsub->publisher.msg_pool; d->msgbuf = buf; + d->subrequest = NULL; fsub->publisher.msg_pool = NULL; if(fsub->publisher.intercept || fsub->publisher.upstream_request_url == NULL) { // don't need to send request upstream - d->subrequest = NULL; websocket_publish_continue(d); } else { From 11b838721d8268e0052f935c1509b14484663cfe Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 03:34:47 -0500 Subject: [PATCH 02/14] Respect client_max_body_size for incoming websocket messages --- src/subscribers/websocket.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/subscribers/websocket.c b/src/subscribers/websocket.c index 64086b91..dad084f9 100644 --- a/src/subscribers/websocket.c +++ b/src/subscribers/websocket.c @@ -662,10 +662,17 @@ static ngx_int_t websocket_publish(full_subscriber_t *fsub, ngx_buf_t *buf, int #endif ngx_int_t rc = NGX_OK; + ngx_http_core_loc_conf_t *clcf; + clcf = ngx_http_get_module_loc_conf(fsub->sub.request, ngx_http_core_module); ws_publish_data_t *d = ngx_palloc(ws_get_msgpool(fsub), sizeof(*d)); + int buflen = buf->end - buf->start; if(d == NULL) { return NGX_ERROR; } + if(buflen < 0 || (clcf->client_max_body_size && clcf->client_max_body_size < buflen)){ + websocket_respond_status(&fsub->sub, NGX_HTTP_FORBIDDEN, NULL, NULL); + return NGX_ERROR; + } d->fsub = fsub; d->binary = binary; //move the msg pool From 4c376234685f032d2d64af5795c816693f295175 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 19:43:24 -0500 Subject: [PATCH 03/14] Fix memory corruption in is_utf8() when block being checked is >16KB --- src/subscribers/websocket.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/subscribers/websocket.c b/src/subscribers/websocket.c index dad084f9..2518bfcc 100644 --- a/src/subscribers/websocket.c +++ b/src/subscribers/websocket.c @@ -1584,7 +1584,7 @@ static void websocket_reading(ngx_http_request_t *r) { static ngx_flag_t is_utf8(ngx_buf_t *buf) { - u_char *p; + u_char *p, *op; size_t n; u_char c, *last; @@ -1606,6 +1606,7 @@ static ngx_flag_t is_utf8(ngx_buf_t *buf) { } last = p + n; + op = p; for (len = 0; p < last; len++) { c = *p; @@ -1618,13 +1619,13 @@ static ngx_flag_t is_utf8(ngx_buf_t *buf) { if (ngx_utf8_decode(&p, last - p) > 0x10ffff) { /* invalid UTF-8 */ if(mmapped) { - munmap(p, n); + munmap(op, n); } return 0; } } if(mmapped) { - munmap(p, n); + munmap(op, n); } return 1; } From 14514b667117042666620acedf78cd886d841332 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 20:22:58 -0500 Subject: [PATCH 04/14] Fix file-cached messages (over 16KB) not getting passed to upstream --- src/util/nchan_fake_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/nchan_fake_request.c b/src/util/nchan_fake_request.c index 62151de4..91e4e29e 100644 --- a/src/util/nchan_fake_request.c +++ b/src/util/nchan_fake_request.c @@ -468,7 +468,7 @@ nchan_fakereq_subrequest_data_t *nchan_requestmachine_request(nchan_requestmachi fakebody_buf->last_buf = 1; fakebody_buf->last_in_chain = 1; fakebody_buf->flush = 1; - fakebody_buf->memory = 1; + //fakebody_buf->memory = 1; //why were file-cached requests (over 16KB) disabled? nchan_adjust_subrequest(sr, NGX_HTTP_POST, &POST_REQUEST_STRING, sr_body, sz); } From 00fdb66740d36c0d2e3757270fc2840db7b1d90a Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 21:35:11 -0500 Subject: [PATCH 05/14] Better handling of client_max_body_size limit for incoming websocket messages --- src/subscribers/websocket.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/subscribers/websocket.c b/src/subscribers/websocket.c index 2518bfcc..8461bdc4 100644 --- a/src/subscribers/websocket.c +++ b/src/subscribers/websocket.c @@ -662,17 +662,10 @@ static ngx_int_t websocket_publish(full_subscriber_t *fsub, ngx_buf_t *buf, int #endif ngx_int_t rc = NGX_OK; - ngx_http_core_loc_conf_t *clcf; - clcf = ngx_http_get_module_loc_conf(fsub->sub.request, ngx_http_core_module); ws_publish_data_t *d = ngx_palloc(ws_get_msgpool(fsub), sizeof(*d)); - int buflen = buf->end - buf->start; if(d == NULL) { return NGX_ERROR; } - if(buflen < 0 || (clcf->client_max_body_size && clcf->client_max_body_size < buflen)){ - websocket_respond_status(&fsub->sub, NGX_HTTP_FORBIDDEN, NULL, NULL); - return NGX_ERROR; - } d->fsub = fsub; d->binary = binary; //move the msg pool @@ -1093,7 +1086,7 @@ static ngx_int_t websocket_perform_handshake(full_subscriber_t *fsub) { static void websocket_reading(ngx_http_request_t *r); -static ngx_buf_t *websocket_inflate_message(full_subscriber_t *fsub, ngx_buf_t *msgbuf, ngx_pool_t *pool) { +static ngx_buf_t *websocket_inflate_message(full_subscriber_t *fsub, ngx_buf_t *msgbuf, ngx_pool_t *pool, uint64_t max, int *result) { #if (NGX_ZLIB) z_stream *strm; int rc; @@ -1122,7 +1115,7 @@ static ngx_buf_t *websocket_inflate_message(full_subscriber_t *fsub, ngx_buf_t * strm = fsub->deflate.zstream_in; - outbuf = nchan_inflate(strm, msgbuf, fsub->sub.request, pool); + outbuf = nchan_inflate(strm, msgbuf, fsub->sub.request, pool, max, result); return outbuf; #else return NULL; @@ -1311,6 +1304,8 @@ static void websocket_reading(ngx_http_request_t *r) { ngx_connection_t *c; ngx_buf_t *msgbuf, buf; //ngx_str_t msg_in_str; + ngx_http_core_loc_conf_t *clcf; + int result; retry: ctx = ngx_http_get_module_ctx(r, ngx_nchan_module); fsub = (full_subscriber_t *)ctx->sub; @@ -1475,8 +1470,15 @@ static void websocket_reading(ngx_http_request_t *r) { return websocket_reading_finalize(r); } - //TODO: check max websocket message length + clcf = ngx_http_get_module_loc_conf(ctx->sub->request, ngx_http_core_module); + if(frame->payload == NULL) { + if(clcf->client_max_body_size && (uint64_t)clcf->client_max_body_size < frame->payload_len) { + websocket_send_close_frame_cstr(fsub, CLOSE_POLICY_VIOLATION, "Message too large."); + ws_destroy_msgpool(fsub); + fsub->publisher.msg_pool = NULL; + return websocket_reading_finalize(r); + } if(ws_get_msgpool(fsub) == NULL) { ERR("failed to get msgpool"); websocket_send_close_frame(fsub, CLOSE_INTERNAL_SERVER_ERROR, NULL); @@ -1493,6 +1495,13 @@ static void websocket_reading(ngx_http_request_t *r) { } set_buffer(&buf, frame->payload, frame->last, frame->payload_len); + if(clcf->client_max_body_size && clcf->client_max_body_size < ngx_buf_size(&buf)) { + websocket_send_close_frame_cstr(fsub, CLOSE_POLICY_VIOLATION, "Message too large."); + ws_destroy_msgpool(fsub); + fsub->publisher.msg_pool = NULL; + return websocket_reading_finalize(r); + } + if (frame->payload_len > 0 && (rc = ws_recv(c, rev, &buf, frame->payload_len)) != NGX_OK) { DBG("ws_recv NOT OK when receiving payload, but that's ok"); @@ -1520,7 +1529,12 @@ static void websocket_reading(ngx_http_request_t *r) { //inflate message if needed if(fsub->deflate.enabled && frame->rsv1) { - if((msgbuf = websocket_inflate_message(fsub, msgbuf, ws_get_msgpool(fsub))) == NULL) { + if((msgbuf = websocket_inflate_message(fsub, msgbuf, ws_get_msgpool(fsub), (uint64_t)clcf->client_max_body_size, &result)) == NULL) { + if(result == -1) { + websocket_send_close_frame_cstr(fsub, CLOSE_POLICY_VIOLATION, "Message too large."); + ws_destroy_msgpool(fsub); + return websocket_reading_finalize(r); + } websocket_send_close_frame_cstr(fsub, CLOSE_INVALID_PAYLOAD, "Invalid permessage-deflate data"); ws_destroy_msgpool(fsub); return websocket_reading_finalize(r); From a5a1352e318014cdefba9ba85b163e67ba80888f Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 21:38:02 -0500 Subject: [PATCH 06/14] Better handling of client_max_body_size limit for incoming websocket messages --- src/util/nchan_util.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/util/nchan_util.c b/src/util/nchan_util.c index f675381e..06fcc0a8 100644 --- a/src/util/nchan_util.c +++ b/src/util/nchan_util.c @@ -964,7 +964,7 @@ ngx_buf_t *nchan_common_deflate(ngx_buf_t *in, ngx_http_request_t *r, ngx_pool_t return out; } -ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, ngx_pool_t *pool) { +ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, ngx_pool_t *pool, uint64_t max, int *result) { ngx_str_t mm_instr = {0, NULL}; int mmapped = 0; ngx_temp_file_t *tf = NULL; @@ -975,6 +975,8 @@ ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, unsigned have = 0; off_t written = 0; int trailer_appended = 0; + + *result = 0; //input if(ngx_buf_in_memory(in)) { @@ -1018,6 +1020,7 @@ ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, } have = ZLIB_CHUNK - stream->avail_out; + if((uint64_t)have + written > max) break; if(stream->avail_out == 0 && tf == NULL) { //if we filled up the buffer, let's start dumping to a file. @@ -1033,6 +1036,11 @@ ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, munmap(mm_instr.data, mm_instr.len); } + if((uint64_t)have + written > max) { + *result = -1; + deflateReset(deflate_zstream); + return NULL; + } if((out = ngx_palloc(pool, sizeof(*out))) == NULL) { nchan_log_request_error(r, "failed to allocate output buf for deflated message"); deflateReset(deflate_zstream); From aece64fca8d525ef4ef305257d49cae01ed0e7c2 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 1 Nov 2020 21:38:38 -0500 Subject: [PATCH 07/14] Better handling of client_max_body_size limit for incoming websocket messages --- src/util/nchan_util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/nchan_util.h b/src/util/nchan_util.h index 3dd2358a..5b132a41 100644 --- a/src/util/nchan_util.h +++ b/src/util/nchan_util.h @@ -70,7 +70,7 @@ ngx_int_t nchan_common_deflate_init(nchan_main_conf_t *mcf); ngx_buf_t *nchan_common_deflate(ngx_buf_t *in, ngx_http_request_t *r, ngx_pool_t *pool); ngx_int_t nchan_common_simple_deflate_raw_block(ngx_str_t *in, ngx_str_t *out); ngx_int_t nchan_common_simple_deflate(ngx_str_t *in, ngx_str_t *out); -ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, ngx_pool_t *pool); +ngx_buf_t *nchan_inflate(z_stream *stream, ngx_buf_t *in, ngx_http_request_t *r, ngx_pool_t *pool, uint64_t max, int *result); uint64_t nchan_htonll(uint64_t value); uint64_t nchan_ntohll(uint64_t value); From 579ac295cbeb7870f20911f38e3378fb997a347f Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 15 Nov 2020 01:32:11 -0500 Subject: [PATCH 08/14] Fix crash when using X-Accel-Redirect with message forwarding --- src/util/nchan_output.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/nchan_output.h b/src/util/nchan_output.h index d465de82..07fcccc4 100644 --- a/src/util/nchan_output.h +++ b/src/util/nchan_output.h @@ -28,3 +28,5 @@ ngx_int_t nchan_msg_buf_open_fd_if_needed(ngx_buf_t *buf, ngx_file_t *file, ngx_ ngx_str_t *msgtag_to_str(nchan_msg_id_t *id); ngx_str_t *msgid_to_str(nchan_msg_id_t *id); size_t msgtag_to_strptr(nchan_msg_id_t *id, char *ch); + +extern ngx_module_t ngx_http_charset_filter_module; From 86eae8b18cd3366022a1bdae157cb6e15d2e2a08 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 15 Nov 2020 01:35:15 -0500 Subject: [PATCH 09/14] Fix crash when using X-Accel-Redirect with message forwarding --- src/util/nchan_output.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util/nchan_output.c b/src/util/nchan_output.c index 1f620280..48973319 100644 --- a/src/util/nchan_output.c +++ b/src/util/nchan_output.c @@ -290,6 +290,11 @@ static ngx_int_t nchan_output_filter_generic(ngx_http_request_t *r, nchan_msg_t if(r->out == NULL) { if(ctx) { flush_all_the_reserved_things(ctx); + + //Fix crash when using X-Accel-Redirect with message forwarding + //See https://github.com/slact/nchan/pull/591 + //There may be a better way to do this... + ngx_http_set_ctx(r->main, NULL, ngx_http_charset_filter_module); } } From 713fbcbef4c13569821c2e583a245cdc72259c72 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 6 Dec 2020 01:19:00 -0500 Subject: [PATCH 10/14] Fix crash when multi-id pubsub is closed by timeout https://github.com/slact/nchan/pull/591 --- src/subscribers/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/subscribers/common.c b/src/subscribers/common.c index 611d9421..50720fb0 100644 --- a/src/subscribers/common.c +++ b/src/subscribers/common.c @@ -287,7 +287,7 @@ void nchan_subscriber_timeout_ev_handler(ngx_event_t *ev) { #if FAKESHARD memstore_fakeprocess_push(sub->owner); #endif - sub->dequeue_after_response = 1; + //sub->dequeue_after_response = 1; //see https://github.com/slact/nchan/pull/591 sub->fn->respond_status(sub, NGX_HTTP_REQUEST_TIMEOUT, &NCHAN_HTTP_STATUS_408, NULL); #if FAKESHARD memstore_fakeprocess_pop(); From d19723c2e0a9964224432321a127c86bc1287ee6 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 6 Dec 2020 01:21:02 -0500 Subject: [PATCH 11/14] Fix crash when multi-id pubsub is closed by HTTP DELETE https://github.com/slact/nchan/pull/591 --- src/subscribers/internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/subscribers/internal.c b/src/subscribers/internal.c index 8f4a8a21..2f0ccf05 100644 --- a/src/subscribers/internal.c +++ b/src/subscribers/internal.c @@ -212,7 +212,7 @@ static ngx_int_t internal_respond_status(subscriber_t *self, ngx_int_t status_co internal_subscriber_t *f = (internal_subscriber_t *)self; DBG("%p status %i", self, status_code); if(status_code == NGX_HTTP_GONE) { - self->dequeue_after_response = 1; + //self->dequeue_after_response = 1; //see https://github.com/slact/nchan/pull/591 } f->respond_status(status_code, (void *)status_line, f->privdata); reset_timer(f); From 035d7c542bf2f874c7b3dd5ca358e168ed3d28a0 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Sun, 6 Dec 2020 01:24:30 -0500 Subject: [PATCH 12/14] Fix crash when forwarding used without proxy_pass https://github.com/slact/nchan/pull/591 --- src/util/nchan_fake_request.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/util/nchan_fake_request.c b/src/util/nchan_fake_request.c index 91e4e29e..f17e3c72 100644 --- a/src/util/nchan_fake_request.c +++ b/src/util/nchan_fake_request.c @@ -374,6 +374,14 @@ static void fakerequest_cleanup_timer_handler(ngx_event_t *ev) { nchan_finalize_fake_request(d->r, NGX_OK); } +//see https://github.com/slact/nchan/pull/591 +typedef struct { + void *fsub; + ngx_pool_t *pool; + ngx_buf_t *msgbuf; + nchan_fakereq_subrequest_data_t *subrequest; +} ws_publish_data_stub_t; + nchan_fakereq_subrequest_data_t *nchan_requestmachine_request(nchan_requestmachine_t *rm, nchan_requestmachine_request_params_t *params) { nchan_fakereq_subrequest_data_t *d; ngx_pool_t *pool = params->pool; @@ -482,6 +490,11 @@ nchan_fakereq_subrequest_data_t *nchan_requestmachine_request(nchan_requestmachi nchan_slist_append(&rm->request_queue, d); + //see https://github.com/slact/nchan/pull/591 + ws_publish_data_stub_t *pd; + pd = (ws_publish_data_stub_t*)(params->pd); + if(pd) pd->subrequest = d; + nchan_requestmachine_run(rm); return d; } From 9dfdc24310d15dcc345c370894d86e850c061154 Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Wed, 24 Feb 2021 18:48:41 -0500 Subject: [PATCH 13/14] Fixed uninitialized result var --- src/subscribers/websocket.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/subscribers/websocket.c b/src/subscribers/websocket.c index 8461bdc4..cbbc3523 100644 --- a/src/subscribers/websocket.c +++ b/src/subscribers/websocket.c @@ -1091,6 +1091,8 @@ static ngx_buf_t *websocket_inflate_message(full_subscriber_t *fsub, ngx_buf_t * z_stream *strm; int rc; ngx_buf_t *outbuf; + + *result = 0; if(fsub->deflate.zstream_in == NULL) { if((fsub->deflate.zstream_in = ngx_calloc(sizeof(*strm), ngx_cycle->log)) == NULL) { From 4bdc0f76558f0c9ab709bed9a90fc025b2091bdf Mon Sep 17 00:00:00 2001 From: sobitcorp <58403851+sobitcorp@users.noreply.github.com> Date: Fri, 26 Feb 2021 19:18:59 -0500 Subject: [PATCH 14/14] Fixed uninitialized result var - attempt 2 --- src/subscribers/websocket.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/subscribers/websocket.c b/src/subscribers/websocket.c index cbbc3523..34055686 100644 --- a/src/subscribers/websocket.c +++ b/src/subscribers/websocket.c @@ -1087,12 +1087,13 @@ static ngx_int_t websocket_perform_handshake(full_subscriber_t *fsub) { static void websocket_reading(ngx_http_request_t *r); static ngx_buf_t *websocket_inflate_message(full_subscriber_t *fsub, ngx_buf_t *msgbuf, ngx_pool_t *pool, uint64_t max, int *result) { + +*result = 0; + #if (NGX_ZLIB) z_stream *strm; int rc; ngx_buf_t *outbuf; - - *result = 0; if(fsub->deflate.zstream_in == NULL) { if((fsub->deflate.zstream_in = ngx_calloc(sizeof(*strm), ngx_cycle->log)) == NULL) {