From 25e5543c7f66c97871d9202cd15b9945de90faf1 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Fri, 23 Nov 2018 22:33:01 -0300 Subject: [PATCH] Allow empty arrays in JSON parser Issue #1576 --- apache2/msc_json.c | 58 ++++++++++++++- tests/regression/rule/15-json.t | 121 ++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 2 deletions(-) diff --git a/apache2/msc_json.c b/apache2/msc_json.c index 3a7a03d7..c3066032 100644 --- a/apache2/msc_json.c +++ b/apache2/msc_json.c @@ -150,6 +150,60 @@ static int yajl_number(void *ctx, const char *value, size_t length) return json_add_argument(msr, value, length); } +static int yajl_start_array(void *ctx) { + modsec_rec *msr = (modsec_rec *) ctx; + + if (!msr->json->current_key && !msr->json->prefix) { + msr->json->prefix = apr_pstrdup(msr->mp, "array"); + msr->json->current_key = apr_pstrdup(msr->mp, "array"); + } + else if (msr->json->prefix) { + msr->json->prefix = apr_psprintf(msr->mp, "%s.%s", msr->json->prefix, + msr->json->current_key); + } + else { + msr->json->prefix = apr_pstrdup(msr->mp, msr->json->current_key); + } + + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "New JSON hash context (prefix '%s')", msr->json->prefix); + } + + + return 1; +} + + +static int yajl_end_array(void *ctx) { + modsec_rec *msr = (modsec_rec *) ctx; + unsigned char *separator = (unsigned char *) NULL; + + /** + * If we have no prefix, then this is the end of a top-level hash and + * we don't do anything + */ + if (msr->json->prefix == NULL) return 1; + + /** + * Current prefix might or not include a separator character; top-level + * hash keys do not have separators in the variable name + */ + separator = strrchr(msr->json->prefix, '.'); + + if (separator) { + msr->json->prefix = apr_pstrmemdup(msr->mp, msr->json->prefix, + separator - msr->json->prefix); + } + else { + /** + * TODO: Check if it is safe to do this kind of pointer tricks + */ + msr->json->prefix = (unsigned char *) NULL; + } + + return 1; +} + /** * Callback for a new hash, which indicates a new subtree, labeled as the current * argument name, is being created @@ -237,8 +291,8 @@ int json_init(modsec_rec *msr, char **error_msg) { yajl_start_map, yajl_map_key, yajl_end_map, - NULL /* yajl_start_array */, - NULL /* yajl_end_array */ + yajl_start_array, + yajl_end_array }; if (error_msg == NULL) return -1; diff --git a/tests/regression/rule/15-json.t b/tests/regression/rule/15-json.t index e75f89c1..181df9e0 100644 --- a/tests/regression/rule/15-json.t +++ b/tests/regression/rule/15-json.t @@ -35,5 +35,126 @@ ), ), ), +}, +{ + type => "rule", + comment => "json parser - issue #1576 - 1", + conf => qq( + SecRuleEngine On + SecRequestBodyAccess On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRule REQUEST_HEADERS:Content-Type "application/json" \\ + "id:'200001',phase:1,t:none,t:lowercase,pass,nolog,ctl:requestBodyProcessor=JSON" + SecRule ARGS "bar" "id:'200441',phase:3,log" + ), + match_log => { + error => [ qr/ModSecurity: Warning. Pattern match "bar" at ARGS:foo.|ModSecurity: JSON support was not enabled/s, 1 ], + debug => [ qr/ARGS:foo|ARGS:mod|ARGS:ops.ops.ops|ARGS:ops.ops.ops|ARGS:ops.ops|ARGS:ops.ops|ARGS:ops.ops.eins.eins|ARGS:ops.ops.eins.eins|ARGS:whee/, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => "application/json", + ], + normalize_raw_request_data( + q( + { + "foo":"bar", + "mod":"sec", + "ops":[ + [ + "um", + "um e meio" + ], + "dois", + "tres", + { + "eins":[ + "zwei", + "drei" + ] + } + ], + "whee":"lhebs" + } + ), + ), + ), +}, +{ + type => "rule", + comment => "json parser - issue #1576 - 2", + conf => qq( + SecRuleEngine On + SecRequestBodyAccess On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRule REQUEST_HEADERS:Content-Type "application/json" \\ + "id:'200001',phase:1,t:none,t:lowercase,pass,nolog,ctl:requestBodyProcessor=JSON" + SecRule ARGS "um" "id:'200441',phase:3,log" + ), + match_log => { + error => [ qr/ModSecurity: Warning. Pattern match "um" at ARGS:array.array|ModSecurity: JSON support was not enabled/s, 1 ], + debug => [ qr/ARGS:array.array|ARGS:array.array/, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => "application/json", + ], + normalize_raw_request_data( + q( + [ + "um", + "um e meio" + ] + ), + ), + ), +}, +{ + type => "rule", + comment => "json parser - issue #1576 - 3", + conf => qq( + SecRuleEngine On + SecRequestBodyAccess On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRule REQUEST_HEADERS:Content-Type "application/json" \\ + "id:'200001',phase:1,t:none,t:lowercase,pass,nolog,ctl:requestBodyProcessor=JSON" + SecRule ARGS "seis" "id:'200441',phase:3,log" + ), + match_log => { + error => [ qr/ModSecurity: Warning. Pattern match "seis" at ARGS:array.array.cinco.|ModSecurity: JSON support was not enabled/s, 1 ], + debug => [ qr/ARGS:array.array|ARGS:array.array|ARGS:array.array.tres|ARGS:array.array.cinco/, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => "application/json", + ], + normalize_raw_request_data( + q( + [ + "um", + "um e meio", + { + "tres": "quatro", + "cinco": "seis" + } + ] + ), + ), + ), }