Skip to content

Commit

Permalink
Fix: "application/reports+json" request parsing as JSON (#155)
Browse files Browse the repository at this point in the history 
* 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"
  • Loading branch information
@lubomudr
lubomudr authored Jul 10, 2024
1 parent aa3b6c7 commit c02bc1d
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 105 deletions.
189 changes: 87 additions & 102 deletions naxsi_src/naxsi_json.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -82,72 +82,72 @@ 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);
}

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;
val.len = 0;

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++;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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 == '{') {
/*
Expand All @@ -224,78 +224,63 @@ 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);
}

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);
}

/*
Expand Down
4 changes: 4 additions & 0 deletions naxsi_src/naxsi_runtime.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
35 changes: 33 additions & 2 deletions unit-tests/tests/14json.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion unit-tests/tests/15json_wl.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit c02bc1d

Please sign in to comment.