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

COMPCOV off-by-one (or more!), index overflowing map #30

Open
AlLongley opened this issue Jan 6, 2025 · 2 comments
Open

COMPCOV off-by-one (or more!), index overflowing map #30

AlLongley opened this issue Jan 6, 2025 · 2 comments

Comments

@AlLongley
Copy link

I have managed to trigger a buffer overflow in my Qiling target if _uc_hook_sub_impl_64 is called with a cur_loc close to, but less than the total MAP_SIZE. I believe the per-byte comparison "+" offsets push it over, incrementing an index beyond the map's buffer itself.

Take for example the following values:

MAP_SIZE = afl_inst_rms_ = 0x10000
cur_loc = 0xffff
afl_prev_loc_ = 0x3777

unicornafl/unicornafl.cpp

Lines 367 to 370 in 2abdcd3

void _uc_hook_sub_impl_64(uint64_t cur_loc, uint64_t arg1, uint64_t arg2) {
if ((arg1 & 0xff00000000000000) == (arg2 & 0xff00000000000000)) {
this->afl_area_ptr_[(cur_loc + 6) ^ this->afl_prev_loc_]++;

The resulting incremented afl_area_ptr_ index is: 0x13772, overflowing the MAP_SIZE by a lot, losing a coverage point and potentially crashing the target.

I can see there's a check to attempt to avoid this before the individual byte checks are called, should the comparison instead be >= (ucafl->afl_inst_rms_-6)) or similar ?

if (unlikely(cur_loc >= ucafl->afl_inst_rms_)) {

@vanhauser-thc
Copy link
Member

thanks for the bug report! I just pushed a simple fix.

@wtdcode
Copy link
Collaborator

wtdcode commented Jan 6, 2025

Nice catch and thanks @vanhauser-thc for the quick fix.

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

No branches or pull requests

3 participants