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

DEX: read_uleb128_bounded(), fix buffer-overflow #1949

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

cblichmann
Copy link
Contributor

This allows us to read ULEB128 encoded values close to the end of a buffer.

This change fixes a heap-buffer-overflow in load_encoded_method(), found by fuzzing. In that method, the fits_in_dex() check is not sufficient, as each uint32_t can occupy up to 5 bytes in ULEB128 encoding.

I did not replace all uses of read_uleb128() in dex.c, but follow-up changes should probably do that.

This allows us to read ULEB128 encoded values close to the end of a
buffer.

This change fixes a heap-buffer-overflow in `load_encoded_method()`,
found by fuzzing. In that method, the `fits_in_dex()` check is not
sufficient, as each `uint32_t` can occupy up to 5 bytes in ULEB128
encoding.

I did not replace all uses of `read_uleb128()` in `dex.c`, but follow-up
changes should probably do that.
@plusvic plusvic merged commit 84f93ac into VirusTotal:master Aug 16, 2023
9 checks passed
cblichmann added a commit to cblichmann/yara that referenced this pull request Aug 17, 2023
This is a follow-up to PR VirusTotal#1949, replacing all remaingin uses of
`read_uleb128()` in `dex.c`.

It also fixes another heap-buffer-overflow, again found by fuzzing.

Tested with `test-dex.c`
plusvic pushed a commit that referenced this pull request Aug 18, 2023
This is a follow-up to PR #1949, replacing all remaingin uses of
`read_uleb128()` in `dex.c`.

It also fixes another heap-buffer-overflow, again found by fuzzing.

Tested with `test-dex.c`
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