From c02bc1dafbf4582f871c78844dd6a2bd7a9d792a Mon Sep 17 00:00:00 2001 From: lubomudr <108034465+lubomudr@users.noreply.github.com> Date: Wed, 10 Jul 2024 22:03:17 +1000 Subject: [PATCH] Fix: "application/reports+json" request parsing as JSON (#155) * Fix: JSON parsing in accordance with RFC 7159 standard and "application/reports+json" request parsing as JSON * Fix: test "json wl 1.1 : match (no variable name)" must return error "Invalid JSON" --- naxsi_src/naxsi_json.c | 189 ++++++++++++++++------------------- naxsi_src/naxsi_runtime.c | 4 + unit-tests/tests/14json.t | 35 ++++++- unit-tests/tests/15json_wl.t | 2 +- 4 files changed, 125 insertions(+), 105 deletions(-) diff --git a/naxsi_src/naxsi_json.c b/naxsi_src/naxsi_json.c index f1853830..660e4e6a 100644 --- a/naxsi_src/naxsi_json.c +++ b/naxsi_src/naxsi_json.c @@ -55,11 +55,11 @@ ngx_http_nx_json_quoted(ngx_json_t* js, ngx_str_t* ve) if (*json_char(js) == '\\') { js->off += 2; if (js->off >= js->len) - break; + return (NGX_ERROR); continue; } if (*json_char(js) == '"') { - vn_end = js->src + js->off; + vn_end = json_char(js); js->off++; break; } @@ -82,29 +82,31 @@ ngx_http_nx_json_quoted(ngx_json_t* js, ngx_str_t* ve) ngx_int_t ngx_http_nx_json_array(ngx_json_t* js) { - ngx_int_t rc; - - js->c = *(js->src + js->off); + ngx_http_nx_json_forward(js); if (js->c != '[' || js->depth > JSON_MAX_DEPTH) return (NGX_ERROR); js->off++; + ngx_http_nx_json_forward(js); + if (js->c == ']') { + /* empty array */ + js->off++; + return (NGX_OK); + } do { - rc = ngx_http_nx_json_val(js); - /* if we cannot extract the value, - we may have reached array end. */ - if (rc != NGX_OK) { - break; + if (ngx_http_nx_json_val(js) != NGX_OK) { + return (NGX_ERROR); } ngx_http_nx_json_forward(js); - if (js->c == ',') { - js->off++; - ngx_http_nx_json_forward(js); - } else + if (js->c != ',') break; - } while (rc == NGX_OK); + js->off++; + } while (js->off < js->len); + + ngx_http_nx_json_forward(js); if (js->c != ']') { return (NGX_ERROR); } + js->off++; return (NGX_OK); } @@ -112,7 +114,6 @@ ngx_int_t ngx_http_nx_json_val(ngx_json_t* js) { ngx_str_t val; - ngx_int_t ret; ngx_str_t empty = ngx_string(""); val.data = NULL; @@ -120,34 +121,33 @@ ngx_http_nx_json_val(ngx_json_t* js) ngx_http_nx_json_forward(js); if (js->c == '"') { - ret = ngx_http_nx_json_quoted(js, &val); - if (ret == NGX_OK) { - /* parse extracted values. */ - if (js->loc_cf->body_rules) { - ngx_http_basestr_ruleset_n( - js->r->pool, &js->ckey, &val, js->loc_cf->body_rules, js->r, js->ctx, BODY); - } - if (js->main_cf->body_rules) { - ngx_http_basestr_ruleset_n( - js->r->pool, &js->ckey, &val, js->main_cf->body_rules, js->r, js->ctx, BODY); - } - NX_DEBUG(_debug_json, - NGX_LOG_DEBUG_HTTP, - js->r->connection->log, - 0, - "quoted-JSON '%V' : '%V'", - &(js->ckey), - &(val)); + if (ngx_http_nx_json_quoted(js, &val) != NGX_OK) { + return (NGX_ERROR); + } + /* parse extracted values. */ + if (js->loc_cf->body_rules) { + ngx_http_basestr_ruleset_n( + js->r->pool, &js->ckey, &val, js->loc_cf->body_rules, js->r, js->ctx, BODY); } - return (ret); + if (js->main_cf->body_rules) { + ngx_http_basestr_ruleset_n( + js->r->pool, &js->ckey, &val, js->main_cf->body_rules, js->r, js->ctx, BODY); + } + NX_DEBUG(_debug_json, + NGX_LOG_DEBUG_HTTP, + js->r->connection->log, + 0, + "quoted-JSON '%V' : '%V'", + &(js->ckey), + &(val)); + return (NGX_OK); } if ((js->c >= '0' && js->c <= '9') || js->c == '-') { - val.data = js->src + js->off; - while (((*(js->src + js->off) >= '0' && *(js->src + js->off) <= '9') || - *(js->src + js->off) == '.' || *(js->src + js->off) == '+' || - *(js->src + js->off) == '-' || *(js->src + js->off) == 'e' || - *(js->src + js->off) == 'E') && - js->off < js->len) { + val.data = json_char(js); + while (js->off < js->len && + ((*json_char(js) >= '0' && *json_char(js) <= '9') || *json_char(js) == '.' || + *json_char(js) == '+' || *json_char(js) == '-' || *json_char(js) == 'e' || + *json_char(js) == 'E')) { val.len++; js->off++; } @@ -169,12 +169,12 @@ ngx_http_nx_json_val(ngx_json_t* js) &(val)); return (NGX_OK); } - if (!strncasecmp((const char*)(js->src + js->off), (const char*)"true", 4) || - !strncasecmp((const char*)(js->src + js->off), (const char*)"false", 5) || - !strncasecmp((const char*)(js->src + js->off), (const char*)"null", 4)) { - js->c = *(js->src + js->off); + if (!strncasecmp((const char*)json_char(js), (const char*)"true", 4) || + !strncasecmp((const char*)json_char(js), (const char*)"false", 5) || + !strncasecmp((const char*)json_char(js), (const char*)"null", 4)) { + js->c = *json_char(js); /* we don't check static values, do we ?! */ - val.data = js->src + js->off; + val.data = json_char(js); if (js->c == 'F' || js->c == 'f') { js->off += 5; val.len = 5; @@ -202,12 +202,12 @@ ngx_http_nx_json_val(ngx_json_t* js) } if (js->c == '[') { - ret = ngx_http_nx_json_array(js); - if (js->c != ']') { + js->depth++; + if (ngx_http_nx_json_array(js) != NGX_OK) { return (NGX_ERROR); } - js->off++; - return (ret); + js->depth--; + return (NGX_OK); } if (js->c == '{') { /* @@ -224,13 +224,19 @@ ngx_http_nx_json_val(ngx_json_t* js) ngx_http_basestr_ruleset_n( js->r->pool, &js->ckey, &empty, js->main_cf->body_rules, js->r, js->ctx, BODY); } - ret = ngx_http_nx_json_obj(js); - ngx_http_nx_json_forward(js); - if (js->c != '}') { + NX_DEBUG(_debug_json, + NGX_LOG_DEBUG_HTTP, + js->r->connection->log, + 0, + "sub-struct-JSON '%V' : {...}", + &(js->ckey)); + + js->depth++; + if (ngx_http_nx_json_obj(js) != NGX_OK) { return (NGX_ERROR); } - js->off++; - return (ret); + js->depth--; + return (NGX_OK); } return (NGX_ERROR); } @@ -238,64 +244,43 @@ ngx_http_nx_json_val(ngx_json_t* js) ngx_int_t ngx_http_nx_json_obj(ngx_json_t* js) { - js->c = *(js->src + js->off); - + ngx_http_nx_json_forward(js); if (js->c != '{' || js->depth > JSON_MAX_DEPTH) return (NGX_ERROR); js->off++; - + ngx_http_nx_json_forward(js); + if (js->c == '}') { + /* empty object */ + js->off++; + return (NGX_OK); + } do { + if (ngx_http_nx_json_quoted(js, &(js->ckey)) != NGX_OK) { + return (NGX_ERROR); + } + if (ngx_http_nx_json_seek(js, ':') != NGX_OK) { + return (NGX_ERROR); + } + js->off++; ngx_http_nx_json_forward(js); - /* check subs (arrays, objects) */ - switch (js->c) { - case '[': /* array */ - js->depth++; - ngx_http_nx_json_array(js); - if (ngx_http_nx_json_seek(js, ']')) - return (NGX_ERROR); - js->off++; - js->depth--; - break; - case '{': /* sub-object */ - js->depth++; - ngx_http_nx_json_obj(js); - if (js->c != '}') { - return (NGX_ERROR); - } - js->off++; - js->depth--; - break; - case '"': /* key : value, extract and parse. */ - if (ngx_http_nx_json_quoted(js, &(js->ckey)) != NGX_OK) { - return (NGX_ERROR); - } - if (ngx_http_nx_json_seek(js, ':')) { - return (NGX_ERROR); - } - js->off++; - ngx_http_nx_json_forward(js); - if (ngx_http_nx_json_val(js) != NGX_OK) { - return (NGX_ERROR); - } + if (ngx_http_nx_json_val(js) != NGX_OK) { + return (NGX_ERROR); } ngx_http_nx_json_forward(js); /* another element ? */ - if (js->c == ',') { - js->off++; - ngx_http_nx_json_forward(js); - continue; - - } else if (js->c == '}') { - js->depth--; - /* or maybe we just finished parsing this object */ - return (NGX_OK); - } else { - /* nothing we expected, die. */ - return (NGX_ERROR); + if (js->c != ',') { + break; } + js->off++; + ngx_http_nx_json_forward(js); } while (js->off < js->len); - return (NGX_ERROR); + ngx_http_nx_json_forward(js); + if (js->c != '}') { + return (NGX_ERROR); + } + js->off++; + return (NGX_OK); } /* diff --git a/naxsi_src/naxsi_runtime.c b/naxsi_src/naxsi_runtime.c index f598af47..50a92a27 100644 --- a/naxsi_src/naxsi_runtime.c +++ b/naxsi_src/naxsi_runtime.c @@ -2732,6 +2732,10 @@ ngx_http_naxsi_body_parse(ngx_http_request_ctx_t* ctx, else if (!ngx_strncasecmp(content_type_str, (u_char*)"application/vnd.api+json", 24)) { ngx_http_naxsi_json_parse(ctx, r, full_body, full_body_len); } + /* 24 = echo -n "application/reports+json" | wc -c */ + else if (!ngx_strncasecmp(content_type_str, (u_char*)"application/reports+json", 24)) { + ngx_http_naxsi_json_parse(ctx, r, full_body, full_body_len); + } /* 22 = echo -n "application/csp-report" | wc -c */ else if (!ngx_strncasecmp(content_type_str, (u_char*)"application/csp-report", 22)) { ngx_http_naxsi_json_parse(ctx, r, full_body, full_body_len); diff --git a/unit-tests/tests/14json.t b/unit-tests/tests/14json.t index 6246cd0d..abd3d1c0 100644 --- a/unit-tests/tests/14json.t +++ b/unit-tests/tests/14json.t @@ -356,7 +356,7 @@ use URI::Escape; }" --- error_code: 412 -=== JSON7 : Valid JSON with empty array item (Extra ',' in array) +=== JSON7 : inValid JSON with empty array item (Extra ',' in array) --- main_config load_module $TEST_NGINX_NAXSI_MODULE_SO; --- http_config @@ -404,7 +404,7 @@ use URI::Escape; } }" ---- error_code: 200 +--- error_code: 412 === JSON8 : valid JSON - too deep ! --- main_config load_module $TEST_NGINX_NAXSI_MODULE_SO; @@ -1013,3 +1013,34 @@ use URI::Escape; } " --- error_code: 200 + +=== JSON19 : Invalid JSON application/reports+json +--- main_config +load_module $TEST_NGINX_NAXSI_MODULE_SO; +--- http_config +include $TEST_NGINX_NAXSI_RULES; +--- config +location / { + SecRulesEnabled; + DeniedUrl "/RequestDenied"; + CheckRule "$SQL >= 8" BLOCK; + CheckRule "$RFI >= 8" BLOCK; + CheckRule "$TRAVERSAL >= 4" BLOCK; + CheckRule "$XSS >= 8" BLOCK; + root $TEST_NGINX_SERVROOT/html/; + index index.html index.htm; + error_page 405 = $uri; +} +location /RequestDenied { + return 412; +} +--- more_headers +Content-Type: application/reports+json +--- request eval +use URI::Escape; +"POST / +[{ + \"name\": \"value\" +] +" +--- error_code: 412 diff --git a/unit-tests/tests/15json_wl.t b/unit-tests/tests/15json_wl.t index 23106ca1..8809a15d 100644 --- a/unit-tests/tests/15json_wl.t +++ b/unit-tests/tests/15json_wl.t @@ -427,7 +427,7 @@ use URI::Escape; [\"there\", \"is\", \"no\", \"way\"] } " ---- error_code: 200 +--- error_code: 412 === json wl 2.0 : malformed json (missing opening {) --- main_config load_module $TEST_NGINX_NAXSI_MODULE_SO;