Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

Fix culprit for JSON heap overflow #2

Closed
wants to merge 1 commit into from
Closed

Fix culprit for JSON heap overflow #2

wants to merge 1 commit into from

Conversation

ex0dus-0x
Copy link

Bounty URL: https://huntr.dev/bounties/1-other-mjs/

Description

Fixes how a heap buffer is allocates and properly copies over parsed JSON string to prevent any out-of-bounds read/writes.

It turns out that the culprit for this didn't exist in json_get_escape_len, but rather the very first allocation that gets made before copying over the parsed JSON string to begin parsing:

https://github.com/cesanta/mjs/blob/4c870e584d2b2a538abcee5307c498cc37e7ef9d/mjs/src/mjs_json.c#L448=L449

Since a string is being copied over, the buffer allocation is made with size len + 1, and after memcpy, a null byte is written to the end.

Proof of Fix

Without the fix, mjs copies over garbage to the buffer for the previously mentioned test case:

$ ./mjs ../out/crash_offbyone
String: {"e":"1\`��;�
Length: 14
  at ../out/crash_offbyone:2
MJS error: invalid JSON string

With the fix and the insertion of the null-byte, the buffer points to the correct string being parsed:

String: {"e":"1\
Length: 8
  at ../out/crash_offbyone:2
MJS error: invalid JSON string

@huntr-helper
Copy link
Member

👋 Hello, @dimonomid. @ex0dus-0x has opened a PR to us with a fix for a potential vulnerability in your repository. To view the vulnerability, please refer to the bounty URL in the first comment, above. If you want this fix in your repository, a PR will automatically open once you comment:

@huntr-helper - LGTM


☎️ Need further support?

Come and join us on our community Discord!


@dimonomid - want more fixes like this?

Copy this snippet into your README.md for more vulnerability fixes in the future:

[![huntr](https://cdn.huntr.dev/huntr_security_badge_mono.svg)](https://huntr.dev)

huntr

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants