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 Leb128 parsing of WasmLoader. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmorin6
Copy link

@cmorin6 cmorin6 commented Aug 23, 2021

I encountered some wasm files that weren't loading properly due to Leb128 parsing.
The current implementation tries parsing the vale from an array of 5 bytes then compute and consume the expected byte size from the reader. The issue came from Leb128 values that where padded with null bytes (probably as some anti-reversing technique) causing the parsing size and consumed size to differ, leading to a misaligned parsing of the following structures and a failure to load the wasm file.

Example:
e2 81 80 80 00 and e2 01 are both "valid" representation of the value 226 in LEB128

I opted to reuse the LEB128 utility already present in Ghidra for the Dwarf parsing that didn't suffer from this issue.

nneonneo added a commit to nneonneo/ghidra-wasm-plugin that referenced this pull request Sep 27, 2021
This was inspired by garrettgu10#3,
where it was discovered that the Leb128 parser may fail to parse
redundantly-encoded numbers correctly. Although that may have been fixed by the
recent changes to Leb128.java, the more pragmatic approach is simply to use the
more battle-tested LEB128 class instead.
cmorin6 pushed a commit to cmorin6/ghidra-wasm-plugin that referenced this pull request Oct 8, 2021
If a function uses the C stack, the first instruction always seems to be a
global.get with the C stack pointer global variable. Therefore, we guess the C
stack global by looking for the global that appears most frequently in an
initial global.get.

Fixes garrettgu10#3.
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.

1 participant