Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix heap buffer overflow #852

Merged
merged 2 commits into from
May 6, 2024
Merged

Fix heap buffer overflow #852

merged 2 commits into from
May 6, 2024

Conversation

sbvoxel
Copy link
Contributor

@sbvoxel sbvoxel commented Apr 30, 2024

Fixes #800

Note that I have a kernel bug which makes ASan fail when address layout randomization is enabled. On top of that, CMake's CHECK_C_COMPILER_FLAG for some reason returns false on my machine for the address sanitizer (among others). So it's a bit of a bother testing this, but I did do so. I just had to force the address sanitizer to be on and disable address layout randomization. Someone else can perhaps test the new test normally, with and without the fix.

I hope the style of test is fine. It's meant to catch the issue when the address sanitizer is enabled. It will simply pass, or at least likely to, if the sanitizer is off.

The first string in the test is the actual string which triggers the bug. The second doesn't, but protects against future bugs in the surrounding area. More strings can be added with ease.

@sbvoxel
Copy link
Contributor Author

sbvoxel commented Apr 30, 2024

I force pushed some styling changes to hopefully make it match the existing codebase. I accidentally wrote the styling in my personal style :)

Feel free to change further.

@sbvoxel sbvoxel force-pushed the oob branch 2 times, most recently from 1a3319e to 7537355 Compare May 1, 2024 13:16
@Alanscut
Copy link
Collaborator

Alanscut commented May 6, 2024

Looks the actions of GCC on linux got a compile warning.

@sbvoxel
Copy link
Contributor Author

sbvoxel commented May 6, 2024

Pushed a missing cast for C compatibility.

Edit: Aww to the wrong commit. One sec.
Edit: Now ready for another CI run.

cJSON.c Outdated Show resolved Hide resolved
@Alanscut Alanscut merged commit 3ef4e4e into DaveGamble:master May 6, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OOB access in parse_string
2 participants