Skip to content

Memory pointer error in JSON sanitization #2344

Open
@marcstern

Description

@marcstern

In json_add_argument(), we calculate the offset of the data to sanitize the following way:
arg->value_origin_offset = value - base_offset;
'value' is a parameter, 'base_offset' is a pointer containing the original string to sanitize.
Usually, 'value' is a pointer in the original string pointed by 'base_offset'.
BUT, when unescaping the string in yajl_do_parse() on line 253, we unescape it in a new string (hand->decodeBuf->data) and pass this new string - that is in a totally different memory space - to the yajl_string() function, passing it to json_add_argument().

Impact: this could lead to a major memory corruption (putting '*' characters in an invalid part of memory)

Solution: we need to pass the original string to yajl_string().
Hopefully, unescaping the string is always shorter, so we can copy the result inline:

- _CC_CHK(hand->callbacks->yajl_string(
                                    hand->ctx, yajl_buf_data(hand->decodeBuf),
                                    yajl_buf_len(hand->decodeBuf)));
+ bufLen = yajl_buf_len(hand->decodeBuf);
+ strcpy(buf, yajl_buf_data(hand->decodeBuf));
+ _CC_CHK(hand->callbacks->yajl_string(hand->ctx, buf, bufLen));

As this modifies the original input string, we need to copy it before parsing it.
In msc_json.c in json_process_chunk():

-    base_offset = buf;
+    base_offset = apr_pstrmemdup(msr->mp, buf, size);
+    if (!base_offset) return -1;

    /* Feed our parser and catch any errors */
-    msr->json->status = yajl_parse(msr->json->handle, buf, size);
+    msr->json->status = yajl_parse(msr->json->handle, base_offset, size);

Metadata

Metadata

Assignees

Labels

2.xRelated to ModSecurity version 2.xbugIt is a confirmed bug

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions