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 overflow is found in upng.c #3143

Open
iwashiira opened this issue May 15, 2024 · 0 comments · May be fixed by #3156
Open

heap buffer overflow is found in upng.c #3143

iwashiira opened this issue May 15, 2024 · 0 comments · May be fixed by #3156

Comments

@iwashiira
Copy link
Contributor

Our fuzzer found heap buffer overflow in upng.c in the current main(1e0953f).

Following is an output of valgrind.
vuln26-patch.png is in vuln26-patch.zip

==39424== Memcheck, a memory error detector
==39424== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==39424== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==39424== Command: ./build/bin/lws-api-test-upng --stdin ../crash/vuln26-patch.png
==39424==
[2024/05/15 12:23:14:0930] U: LWS UPNG test tool
==39424== Invalid write of size 1
==39424==    at 0x1193DC: unfilter_scanline (upng.c:209)
==39424==    by 0x11977E: lws_upng_emit_next_line (upng.c:269)
==39424==    by 0x113575: main (main.c:97)
==39424==  Address 0x4f85260 is 0 bytes after a block of size 33,280 alloc'd
==39424==    at 0x48487A9: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==39424==    by 0x1645DD: _realloc (alloc.c:151)
==39424==    by 0x16467F: lws_realloc (alloc.c:207)
==39424==    by 0x11A1A3: lws_upng_decode (upng.c:496)
==39424==    by 0x1195B7: lws_upng_emit_next_line (upng.c:249)
==39424==    by 0x113575: main (main.c:97)
==39424==
[2024/05/15 12:23:14:1709] N: main: wrote 0
[2024/05/15 12:23:14:1732] E: unfilter_scanline: line start is broken 125
[2024/05/15 12:23:14:1738] E: main: emit returned FATAL 13
[2024/05/15 12:23:14:1752] U: Completed: FAIL
==39424==
==39424== HEAP SUMMARY:
==39424==     in use at exit: 0 bytes in 0 blocks
==39424==   total heap usage: 7 allocs, 7 frees, 41,084 bytes allocated
==39424==
==39424== All heap blocks were freed -- no leaks are possible
==39424==
==39424== For lists of detected and suppressed errors, rerun with: -s
==39424== ERROR SUMMARY: 8 errors from 1 contexts (suppressed: 0 from 0)

u->u.bypp = (u->u.bpp + 7) / 8;
u->inf.bypl = u->u.bypl = u->width * u->u.bypp;

The code is built on the assumption that bypl > bypp, but by specifying width = 0, we can create a situation where bypp < bypl.

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

u->inf.out = (uint8_t *)lws_malloc(ims, __func__);

While the size of the heap area is specified using bypl as above, some writes are done based on bypp, resulting in a heap BOF.

For example, in the following code flow.

u->u.lines = u->inf.out + u->inf.info_size;

obp = uf->alt ? uf->bypl : 0;
uf->precon = uf->alt ? uf->lines : uf->lines + uf->bypl;
uf->recon = &uf->lines[obp];

for (i = 0; i < uf->bypp; i++)
uf->recon[i] = (uint8_t)(u->inf.out[(uf->sp + i) %
u->inf.info_size]);

OOB happens with the writing of uf->recon[i]

Also, the PR of #3134 was found to be inadequate as a check for interger overflow.
For example, when bypl * 2 causes interger overflow, heap BOF occurs as when width = 0

The bypp check is in the determine_format function, so we need a check for width and a more rigorous check for integer overflow in the ims calculation.

static lws_upng_format_t
determine_format(lws_upng_t* upng) {

Ricerca Security, Inc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant