Skip to content

Avoid overflow in SStream.c#1381

Merged
aquynh merged 1 commit intocapstone-engine:nextfrom
catenacyber:sstreamoverflow
Feb 15, 2019
Merged

Avoid overflow in SStream.c#1381
aquynh merged 1 commit intocapstone-engine:nextfrom
catenacyber:sstreamoverflow

Conversation

@catenacyber
Copy link
Copy Markdown
Contributor

Found by oss-fuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12988

vsnprintf may return more characters than the buffer size
So we must check for overflow...
Finally WASM has made up a disassembly more than 512 characters long...

@aquynh aquynh merged commit 3d25925 into capstone-engine:next Feb 15, 2019
@aquynh
Copy link
Copy Markdown
Collaborator

aquynh commented Feb 15, 2019

merged, thanks!

i still think it is a good idea to keep it this way, so we can catch similar bugs ;-)

@catenacyber
Copy link
Copy Markdown
Contributor Author

So, should we abort rather than return ?
see discussion in #1372 about changing how WASM handles this "switch" jump table

@aquynh
Copy link
Copy Markdown
Collaborator

aquynh commented Feb 15, 2019

but this is a framework, not a program, so we should report error, but never abort.

actually by doing this return, we just silence the bug, which may not be a good idea.

@catenacyber
Copy link
Copy Markdown
Contributor Author

Indeed, so how should we report the bug ?
We can replace the start of the buffer with some string such as invalid... What do you think ?

@aquynh
Copy link
Copy Markdown
Collaborator

aquynh commented Feb 15, 2019

No, i think this is the wrong place to catch/report bugs, but we should detect invalid input long before we reach this function. This works well for all architectures, so this arch should not be an exception.

@catenacyber
Copy link
Copy Markdown
Contributor Author

This input is not invalid...
I am more and more convinced that solution to #1372 is not to list every target of the switch...

@catenacyber catenacyber deleted the sstreamoverflow branch February 16, 2019 08:22
catenacyber added a commit to catenacyber/capstone that referenced this pull request Mar 1, 2019
@riptl riptl mentioned this pull request Jul 22, 2022
6 tasks
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.

2 participants