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

heap buffer over-read in upng-gzip.c #3133

Closed
iwashiira opened this issue May 9, 2024 · 2 comments
Closed

heap buffer over-read in upng-gzip.c #3133

iwashiira opened this issue May 9, 2024 · 2 comments

Comments

@iwashiira
Copy link
Contributor

iwashiira commented May 9, 2024

Our fuzzer found integer overflow in upng.c.in the current main(9ba1504).
This integer overflow cause heap buffer over-read in upng-gzip.c

Following is an output of valgrind.
vuln25.png is in vuln25.zip

==18507== Memcheck, a memory error detector
==18507== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==18507== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==18507== Command: ./build/bin/lws-api-test-upng --stdin ../crash/vuln25.png
==18507==
[2024/05/09 12:19:30:5788] U: LWS UPNG test tool
==18507== Invalid read of size 1
==18507==    at 0x16E646: _lws_upng_inflate_data (upng-gzip.c:767)
==18507==    by 0x11A53C: lws_upng_decode (upng.c:575)
==18507==    by 0x1195B7: lws_upng_emit_next_line (upng.c:249)
==18507==    by 0x113575: main (main.c:97)
==18507==  Address 0x4f850a3 is 1,811 bytes inside an unallocated block of size 4,154,960 in arena "client"
==18507==
[2024/05/09 12:19:30:6551] E: _lws_upng_inflate_data: invalid dist 31
[2024/05/09 12:19:30:6566] E: main: emit returned FATAL 11
[2024/05/09 12:19:30:6581] U: Completed: FAIL
==18507==
==18507== HEAP SUMMARY:
==18507==     in use at exit: 0 bytes in 0 blocks
==18507==   total heap usage: 7 allocs, 7 frees, 38,750 bytes allocated
==18507==
==18507== All heap blocks were freed -- no leaks are possible
==18507==
==18507== For lists of detected and suppressed errors, rerun with: -s
==18507== ERROR SUMMARY: 10 errors from 1 contexts (suppressed: 0 from 0)

It is caused by this line.

size_t ims = (u->u.bypl * 2) + u->inf.info_size;

When an integer overflow occurs at bypl * 2 + u->inf.info_size, then ims < info_size.
Hence when info_size > virt > ims, the check below can be bypassed.
if (virt >= inf->info_size)
lwsl_err("virt %d\n", (int)virt);

Then heap buffer over-read occurs at the following location.
inf->out[inf->outpos++] = inf->out[virt];

Ricerca Security, Inc.

@iwashiira iwashiira changed the title heap buffer over-read in upng-gzip.c and heap buffer overflow in upng.c heap buffer over-read in upng-gzip.c May 9, 2024
@iwashiira
Copy link
Contributor Author

The buffer overflow was for another reason and will be reported in another issue.

@lws-team
Copy link
Member

lws-team commented May 9, 2024

Thanks a lot for the report and patch, it's on main.

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 a pull request may close this issue.

2 participants