owasp-modsecurity / owasp-modsecurity/ModSecurity
Memory pointer error in JSON sanitization
@martinhsv is already working on this.
Since Jul 9, 2020.
- Dominant language
- C++
- Stars
- 9.8k
- Forks
- 1.8k
- Avg merge
- 2h 46m
- Merged PRs (30d)
- 1
Description
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);
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.