Skip to content

Commit 683ba1f

Browse files
authored
Merge pull request #2644 from martinhsv/v2/master
Support configurable limit on depth of JSON parsing
2 parents 199cf5d + 4191833 commit 683ba1f

File tree

6 files changed

+135
-2
lines changed

6 files changed

+135
-2
lines changed

CHANGES

+6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
DD mmm YYYY - 2.9.x (to be released)
2+
-------------------
3+
4+
* Support configurable limit on depth of JSON parsing
5+
[@theMiddleBlue, @airween, @dune73, @martinhsv]
6+
17
21 Jun 2021 - 2.9.4
28
-------------------
39

apache2/apache2_config.c

+30
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ void *create_directory_config(apr_pool_t *mp, char *path)
5050
dcfg->reqbody_inmemory_limit = NOT_SET;
5151
dcfg->reqbody_limit = NOT_SET;
5252
dcfg->reqbody_no_files_limit = NOT_SET;
53+
dcfg->reqbody_json_depth_limit = NOT_SET;
5354
dcfg->resbody_access = NOT_SET;
5455

5556
dcfg->debuglog_name = NOT_SET_P;
@@ -332,6 +333,8 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child)
332333
? parent->reqbody_limit : child->reqbody_limit);
333334
merged->reqbody_no_files_limit = (child->reqbody_no_files_limit == NOT_SET
334335
? parent->reqbody_no_files_limit : child->reqbody_no_files_limit);
336+
merged->reqbody_json_depth_limit = (child->reqbody_json_depth_limit == NOT_SET
337+
? parent->reqbody_json_depth_limit : child->reqbody_json_depth_limit);
335338
merged->resbody_access = (child->resbody_access == NOT_SET
336339
? parent->resbody_access : child->resbody_access);
337340

@@ -648,6 +651,7 @@ void init_directory_config(directory_config *dcfg)
648651
dcfg->reqbody_inmemory_limit = REQUEST_BODY_DEFAULT_INMEMORY_LIMIT;
649652
if (dcfg->reqbody_limit == NOT_SET) dcfg->reqbody_limit = REQUEST_BODY_DEFAULT_LIMIT;
650653
if (dcfg->reqbody_no_files_limit == NOT_SET) dcfg->reqbody_no_files_limit = REQUEST_BODY_NO_FILES_DEFAULT_LIMIT;
654+
if (dcfg->reqbody_json_depth_limit == NOT_SET) dcfg->reqbody_json_depth_limit = REQUEST_BODY_JSON_DEPTH_DEFAULT_LIMIT;
651655
if (dcfg->resbody_access == NOT_SET) dcfg->resbody_access = 0;
652656
if (dcfg->of_limit == NOT_SET) dcfg->of_limit = RESPONSE_BODY_DEFAULT_LIMIT;
653657
if (dcfg->if_limit_action == NOT_SET) dcfg->if_limit_action = REQUEST_BODY_LIMIT_ACTION_REJECT;
@@ -1920,6 +1924,24 @@ static const char *cmd_request_body_no_files_limit(cmd_parms *cmd, void *_dcfg,
19201924
return NULL;
19211925
}
19221926

1927+
static const char *cmd_request_body_json_depth_limit(cmd_parms *cmd, void *_dcfg,
1928+
const char *p1)
1929+
{
1930+
directory_config *dcfg = (directory_config *)_dcfg;
1931+
long int limit;
1932+
1933+
if (dcfg == NULL) return NULL;
1934+
1935+
limit = strtol(p1, NULL, 10);
1936+
if ((limit == LONG_MAX)||(limit == LONG_MIN)||(limit <= 0)) {
1937+
return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRequestBodyJsonDepthLimit: %s", p1);
1938+
}
1939+
1940+
dcfg->reqbody_json_depth_limit = limit;
1941+
1942+
return NULL;
1943+
}
1944+
19231945
static const char *cmd_request_body_access(cmd_parms *cmd, void *_dcfg,
19241946
const char *p1)
19251947
{
@@ -3553,6 +3575,14 @@ const command_rec module_directives[] = {
35533575
"maximum request body size ModSecurity will accept, but excluding the size of uploaded files."
35543576
),
35553577

3578+
AP_INIT_TAKE1 (
3579+
"SecRequestBodyJsonDepthLimit",
3580+
cmd_request_body_json_depth_limit,
3581+
NULL,
3582+
CMD_SCOPE_ANY,
3583+
"maximum request body JSON parsing depth ModSecurity will accept."
3584+
),
3585+
35563586
AP_INIT_TAKE1 (
35573587
"SecRequestEncoding",
35583588
cmd_request_encoding,

apache2/modsecurity.h

+2
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ typedef struct msc_parm msc_parm;
9595
#define REQUEST_BODY_DEFAULT_INMEMORY_LIMIT 131072
9696
#define REQUEST_BODY_DEFAULT_LIMIT 134217728
9797
#define REQUEST_BODY_NO_FILES_DEFAULT_LIMIT 1048576
98+
#define REQUEST_BODY_JSON_DEPTH_DEFAULT_LIMIT 10000
9899
#define RESPONSE_BODY_DEFAULT_LIMIT 524288
99100
#define RESPONSE_BODY_HARD_LIMIT 1073741824L
100101

@@ -498,6 +499,7 @@ struct directory_config {
498499
long int reqbody_inmemory_limit;
499500
long int reqbody_limit;
500501
long int reqbody_no_files_limit;
502+
long int reqbody_json_depth_limit;
501503
int resbody_access;
502504

503505
long int of_limit;

apache2/msc_json.c

+26-2
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ static int yajl_start_array(void *ctx) {
164164
else {
165165
msr->json->prefix = apr_pstrdup(msr->mp, msr->json->current_key);
166166
}
167+
msr->json->current_depth++;
168+
if (msr->json->current_depth > msr->txcfg->reqbody_json_depth_limit) {
169+
msr->json->depth_limit_exceeded = 1;
170+
return 0;
171+
}
167172

168173
if (msr->txcfg->debuglog_level >= 9) {
169174
msr_log(msr, 9, "New JSON hash context (prefix '%s')", msr->json->prefix);
@@ -200,6 +205,7 @@ static int yajl_end_array(void *ctx) {
200205
*/
201206
msr->json->prefix = (unsigned char *) NULL;
202207
}
208+
msr->json->current_depth--;
203209

204210
return 1;
205211
}
@@ -229,6 +235,11 @@ static int yajl_start_map(void *ctx)
229235
else {
230236
msr->json->prefix = apr_pstrdup(msr->mp, msr->json->current_key);
231237
}
238+
msr->json->current_depth++;
239+
if (msr->json->current_depth > msr->txcfg->reqbody_json_depth_limit) {
240+
msr->json->depth_limit_exceeded = 1;
241+
return 0;
242+
}
232243

233244
if (msr->txcfg->debuglog_level >= 9) {
234245
msr_log(msr, 9, "New JSON hash context (prefix '%s')", msr->json->prefix);
@@ -270,6 +281,7 @@ static int yajl_end_map(void *ctx)
270281
msr->json->current_key = msr->json->prefix;
271282
msr->json->prefix = (unsigned char *) NULL;
272283
}
284+
msr->json->current_depth--;
273285

274286
return 1;
275287
}
@@ -308,6 +320,9 @@ int json_init(modsec_rec *msr, char **error_msg) {
308320
msr->json->prefix = (unsigned char *) NULL;
309321
msr->json->current_key = (unsigned char *) NULL;
310322

323+
msr->json->current_depth = 0;
324+
msr->json->depth_limit_exceeded = 0;
325+
311326
/**
312327
* yajl initialization
313328
*
@@ -337,7 +352,11 @@ int json_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
337352
msr->json->status = yajl_parse(msr->json->handle, buf, size);
338353
if (msr->json->status != yajl_status_ok) {
339354
/* We need to free the yajl error message later, how to do this? */
340-
*error_msg = yajl_get_error(msr->json->handle, 0, buf, size);
355+
if (msr->json->depth_limit_exceeded) {
356+
*error_msg = "JSON depth limit exceeded";
357+
} else {
358+
*error_msg = yajl_get_error(msr->json->handle, 0, NULL, 0);
359+
}
341360
return -1;
342361
}
343362

@@ -357,7 +376,12 @@ int json_complete(modsec_rec *msr, char **error_msg) {
357376
msr->json->status = yajl_complete_parse(msr->json->handle);
358377
if (msr->json->status != yajl_status_ok) {
359378
/* We need to free the yajl error message later, how to do this? */
360-
*error_msg = yajl_get_error(msr->json->handle, 0, NULL, 0);
379+
if (msr->json->depth_limit_exceeded) {
380+
*error_msg = "JSON depth limit exceeded";
381+
} else {
382+
*error_msg = yajl_get_error(msr->json->handle, 0, NULL, 0);
383+
}
384+
361385
return -1;
362386
}
363387

apache2/msc_json.h

+2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ struct json_data {
4040
/* prefix is used to create data hierarchy (i.e., 'parent.child.value') */
4141
unsigned char *prefix;
4242
unsigned char *current_key;
43+
long int current_depth;
44+
int depth_limit_exceeded;
4345
};
4446

4547
/* Functions */

tests/regression/rule/15-json.t

+69
Original file line numberDiff line numberDiff line change
@@ -156,5 +156,74 @@
156156
),
157157
),
158158
),
159+
},
160+
{
161+
type => "rule",
162+
comment => "json parser - parsing depth not exceeded",
163+
conf => qq(
164+
SecRuleEngine On
165+
SecRequestBodyAccess On
166+
SecDebugLog $ENV{DEBUG_LOG}
167+
SecDebugLogLevel 9
168+
SecRequestBodyJsonDepthLimit 5
169+
SecRule REQUEST_HEADERS:Content-Type "application/json" \\
170+
"id:'200001',phase:1,t:none,t:lowercase,pass,nolog,ctl:requestBodyProcessor=JSON"
171+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200442',phase:2,log,deny,status:403,msg:'Failed to parse request body'"
172+
),
173+
match_log => {
174+
debug => [ qr/key/s, 1 ],
175+
},
176+
match_response => {
177+
status => qr/^200$/,
178+
},
179+
request => new HTTP::Request(
180+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
181+
[
182+
"Content-Type" => "application/json",
183+
],
184+
normalize_raw_request_data(
185+
q(
186+
{
187+
"key1":{"key2":{"key3":{"key4":{"key5":"thevalue"}}}}
188+
}
189+
),
190+
),
191+
),
192+
},
193+
{
194+
type => "rule",
195+
comment => "json parser - parsing depth exceeded",
196+
conf => qq(
197+
SecRuleEngine On
198+
SecRequestBodyAccess On
199+
SecDebugLog $ENV{DEBUG_LOG}
200+
SecAuditEngine RelevantOnly
201+
SecAuditLog "$ENV{AUDIT_LOG}"
202+
SecDebugLogLevel 9
203+
SecRequestBodyJsonDepthLimit 3
204+
SecRule REQUEST_HEADERS:Content-Type "application/json" \\
205+
"id:'200001',phase:1,t:none,t:lowercase,pass,nolog,ctl:requestBodyProcessor=JSON"
206+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200443',phase:2,log,deny,status:403,msg:'Failed to parse request body'"
207+
),
208+
match_log => {
209+
audit => [ qr/JSON parsing error: JSON depth limit exceeded/s, 1 ],
210+
},
211+
match_response => {
212+
status => qr/^403$/,
213+
},
214+
request => new HTTP::Request(
215+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
216+
[
217+
"Content-Type" => "application/json",
218+
],
219+
normalize_raw_request_data(
220+
q(
221+
{
222+
"key1":{"key2":{"key3":{"key4":{"key5":"thevalue"}}}}
223+
}
224+
),
225+
),
226+
),
159227
}
160228

229+

0 commit comments

Comments
 (0)