libpng
libpng copied to clipboard
glennrp
[BUG]: Nonfatal Error caused by use-of-uninitialized-value（commit ID: a37d483）
Crash Inputs
Here are the files (2 in total) that trigger the error- crashes.zip
Bug Description
I apply MSan (Memory Sanitizer ) to check for memory errors and the error report is as follows.
==120623==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7fa330a6a95d in png_read_filter_row_paeth_multibyte_pixel ./libpng/pngrutil.c:4079:11
#1 0x7fa330a66254 in png_read_filter_row ./libpng/pngrutil.c:4140:7
#2 0x7fa330a15371 in png_read_row ./libpng/pngread.c:548:10
#3 0x49847d in read_png ./libpng/contrib/tools/pngfix.c:3636:15
#4 0x496302 in one_file ./libpng/contrib/tools/pngfix.c:3667:12
#5 0x496302 in main ./libpng/contrib/tools/pngfix.c:4009:16
#6 0x7fa33043b082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
#7 0x41c2cd in _start (./install/libpng/install-msan/bin/pngfix+0x41c2cd)
SUMMARY: MemorySanitizer: use-of-uninitialized-value ./libpng/pngrutil.c:4079:11 in png_read_filter_row_paeth_multibyte_pixel
Steps to Reproduce
- Download the libpng source code with the official link and build it with MSan (-fsanitize=memory)
- Executing pngfix with the provided input files
Hi @WorldExecute , I was able to recreate the bug thanks to your files:
==3961==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7fb76fa4788c in png_read_filter_row_paeth_multibyte_pixel ... /pngrutil.c:4085:11
I'll try to see if I can find the issue and fix it. Would you be happy to verify it once applied?
Hi @WorldExecute ,
could you show me exactly the content of the line ./libpng/pngrutil.c:4079 since I'm getting the error on a different one?
Thanks
Hi @WorldExecute ,
a very quick and dirty fix could be inside png_read_start_row to add the following code.
if ( png_ptr->big_row_buf != NULL)
memset(png_ptr->big_row_buf, 0x00, row_bytes + 48);
if(png_ptr->big_prev_row != NULL)
memset(png_ptr->big_prev_row, 0x00, row_bytes + 48);
It could be added after
if (png_ptr->interlaced != 0)
png_ptr->big_row_buf = (png_bytep)png_calloc(png_ptr,
row_bytes + 48);
else
png_ptr->big_row_buf = (png_bytep)png_malloc(png_ptr, row_bytes + 48);
png_ptr->big_prev_row = (png_bytep)png_malloc(png_ptr, row_bytes + 48);
In my opinion it is due to the fact that png_malloc just calls malloc but don't initialize the memory. I can work on that and improve it. It would really help to avoid similar issues in the future.
Thanks for reporting it
EDIT
Another solution could be in the issue link down below. As of today it is still just a work in progress
Hi @WorldExecute , could you show me exactly the content of the line
./libpng/pngrutil.c:4079since I'm getting the error on a different one?Thanks
The content of the line ./libpng/pngrutil.c:4079 is as follows:
4079: if (pb < pa)
{
pa = pb; a = b;
}
if (pc < pa) a = c;
a += *row;
*row++ = (png_byte)a;
And the code in ./libpng/pngrutil.c:4140 is
4140: pp->read_filter[filter-1](row_info, row, prev_row);
Thanks for your reply!
Hi @WorldExecute ,
perfect thanks, I think I have indentified the problem. Is it possible for you to try the small update for png_malloc that was linked right above your answer please?
Hopefully it will fix this bug :)
Thanks again
Hi @WorldExecute , perfect thanks, I think I have indentified the problem. Is it possible for you to try the small update for
png_mallocthat was linked right above your answer please?Hopefully it will fix this bug :)
Thanks again
After my re-verification, it has been fixed！ Thank you for your time！
Hi @WorldExecute , thanks for the confirmation. Would you mind to leave the issue open for now please? No PR have been submitted nor merge so far.
Thanks
@ctruta I have worked on a patch for this and would like to submit a PR. Are you open for this?
@thealberto yes please!
BTW, if your code changes are integrated into the core of the libpng library (and particularly into any of the png*.c and png*.h files), you will become a Contributing Author. For this reason, please update the AUTHORS file accordingly in your commit.
@ctruta I have just submitted the PR. Let me know what you think about it.
Thanks
Thank you @thealberto, I just wrote my code review. Your fix is nice and elegant, but, unfortunately, it flatly downgrades the performance everywhere and we cannot afford it for that reason.
A more specific fix, either in libpng, or in pngfix, is needed.
@jbowler, FYI.
See my comment here: https://github.com/glennrp/libpng/pull/446#issuecomment-1315663412
Information required:
- Analysis; all the local variables have been initialized, but what is the fuzzer complaining about because the variables have four separate memory locations in them.
- Test case; the PNG file that is producing the report. I'm guessing it is an interlaced file with more than 2 bytes per pixel, but I don't know.
- The pass number, the row number and the pixel offset. Putting an fprintf in for the relevant values should not change the report.
Here's a patch that should make it clearer what is being complained about. It is actually an optimization of the code (it removes the copy of the filter byte which causes a spurious cache load on most modern processors).
pngrutil-mempatch-001.dif.justforgithub.txt
So that patch leaves one more byte in prev_row uninitialized; it should never get touched by the code. A test case would allow a memory checkpoint to be placed on the first 16 bytes of prev_row because these should never be touched. "make check" passes with this change.
Thank you @thealberto, I just wrote my code review. Your fix is nice and elegant, but, unfortunately, it flatly downgrades the performance everywhere and we cannot afford it for that reason.
The change to pngmem.c just changes what png_malloc does to what png_calloc does.
It's easier to change the relevant lines to call png_calloc; it's a one character change rather than an "elegant" change to alter an existing internal API. #define png_malloc png_calloc.
I guess it's good-guy bad-guy here.
If there really is a bug here it is a serious bug; reading uninit memory may seem minor, given that the memory is there, but if it is a bug in libpng it would apparently reveal 1 pixel of bytes from the program memory and it would actually be possible to exploit with a carefully crafted PNG. 1 pixel in this case is 8 bytes.
EDIT: 7 bytes if my analysis so far is correct, but it would be 7 bytes three or four times per (very carefully crafted) PNG. So it might be possible to obtain substantial parts of an app memory. Most likely it is a bug in the fuzzer; the code already uses png_calloc at one point when dealing with interlaced images. The change seems to have happened prior to libpng 1.6 and, of course, there are no comments in the code! "Here be dragons."
Hi @jbowler,
I spent some time as well looking at the bug:
The png_malloc function is basically a wrapper around malloc and does not return an initilised memory.
The bug is indicated to be inside png_read_filter_row_paeth_multibyte_pixel. IMO it is triggered when row which refers to ( png_ptr->prev_row ) is used and bpp > 1.
I made some inline comments inside png_read_start_row
#ifdef PNG_MAX_MALLOC_64K
if (row_bytes > (png_uint_32)65536L)
png_error(png_ptr, "This image requires a row greater than 64KB");
#endif
if (row_bytes + 48 > png_ptr->old_big_row_buf_size)
{
png_free(png_ptr, png_ptr->big_row_buf);
png_free(png_ptr, png_ptr->big_prev_row);
if (png_ptr->interlaced != 0)
png_ptr->big_row_buf = (png_bytep)png_calloc(png_ptr,
row_bytes + 48);
else
png_ptr->big_row_buf = (png_bytep)png_malloc(png_ptr, row_bytes + 48); // <<== buffer allocation
png_ptr->big_prev_row = (png_bytep)png_malloc(png_ptr, row_bytes + 48); // <<== buffer allocation
#ifdef PNG_ALIGNED_MEMORY_SUPPORTED
/* Use 16-byte aligned memory for row_buf with at least 16 bytes
* of padding before and after row_buf; treat prev_row similarly.
* NOTE: the alignment is to the start of the pixels, one beyond the start
* of the buffer, because of the filter byte. Prior to libpng 1.5.6 this
* was incorrect; the filter byte was aligned, which had the exact
* opposite effect of that intended.
*/
{
png_bytep temp = png_ptr->big_row_buf + 32;
size_t extra = (size_t)temp & 0x0f;
png_ptr->row_buf = temp - extra - 1/*filter byte*/;
temp = png_ptr->big_prev_row + 32;
extra = (size_t)temp & 0x0f; // this could be 0 for instance
png_ptr->prev_row = temp - extra - 1/*filter byte*/; // <<== prev_row is png_ptr->big_prev_row + 32
}
#else
/* Use 31 bytes of padding before and 17 bytes after row_buf. */
png_ptr->row_buf = png_ptr->big_row_buf + 31;
png_ptr->prev_row = png_ptr->big_prev_row + 31;
#endif
png_ptr->old_big_row_buf_size = row_bytes + 48;
}
...
memset(png_ptr->prev_row, 0, png_ptr->rowbytes + 1); // <<== I don't think the first 31 bytes are initialised
A small example hopefully can better clarify my idea is the following:
#include <png.h>
#include <string.h>
#define ROW_BYTES 52
int main() {
char *prev_row = NULL;
png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING,NULL, NULL, NULL);
char *big_prev_row = png_malloc(png_ptr, ROW_BYTES + 48);
png_bytep temp = big_prev_row + 32;
size_t extra = (size_t)temp & 0x0f;
prev_row = temp - extra - 1;
printf("big_prev_row: %p\n", big_prev_row);
printf("prev_row: %p\n", prev_row);
printf("distance between prev_row and big_prev_row: %ld\n", prev_row - big_prev_row);
memset(prev_row, 0, ROW_BYTES + 1);
}
Execution
➜ /tmp gcc png.c -o png -lpng && ./png
big_prev_row: 0x565363de57c0
prev_row: 0x565363de57df
distance between prev_row and big_prev_row: 31
➜ /tmp
Continue the execution the png_read_filter_row_paeth_multibyte_pixel is called an png_ptr->prev_row is passed as third parameter
static void
png_read_filter_row_paeth_multibyte_pixel(png_row_infop row_info, png_bytep row,
png_const_bytep prev_row)
{
unsigned int bpp = (row_info->pixel_depth + 7) >> 3; // <<== maybe it could be a positive number?
png_bytep rp_end = row + bpp;
/* Process the first pixel in the row completely (this is the same as 'up'
* because there is only one candidate predictor for the first row).
*/
while (row < rp_end)
{
int a = *row + *prev_row++;
*row++ = (png_byte)a;
}
/* Remainder */
rp_end = rp_end + (row_info->rowbytes - bpp);
while (row < rp_end)
{
int a, b, c, pa, pb, pc, p;
c = *(prev_row - bpp); // with bpp > 1 I think we could read uninitialised memory
a = *(row - bpp);
b = *prev_row++;
p = b - c; // it depends on c
pc = a - c; // it depends on c
#ifdef PNG_USE_ABS
pa = abs(p);
pb = abs(pc);
pc = abs(p + pc);
#else
pa = p < 0 ? -p : p; // it depends on p which depends on c
pb = pc < 0 ? -pc : pc;
pc = (p + pc) < 0 ? -(p + pc) : p + pc;
#endif
if (pb < pa) // bug is triggered
{
pa = pb; a = b;
}
if (pc < pa) a = c;
a += *row;
*row++ = (png_byte)a;
}
}
Finally, I agree with the report and I don't think this is a wrongly reported issue.
What's your opinion on this?
Thanks
Ok, so try this patch (which I will also attach):
diff --git a/pngrutil.c b/pngrutil.c
index ca060dd15..988d832ae 100644
--- a/pngrutil.c
+++ b/pngrutil.c
@@ -4044,6 +4044,9 @@ static void
png_read_filter_row_paeth_multibyte_pixel(png_row_infop row_info, png_bytep row,
png_const_bytep prev_row)
{
+ const png_const_bytep prev_row_start = prev_row;
+ const png_const_bytep row_start = row;
+
unsigned int bpp = (row_info->pixel_depth + 7) >> 3;
png_bytep rp_end = row + bpp;
@@ -4063,6 +4066,16 @@ png_read_filter_row_paeth_multibyte_pixel(png_row_infop row_info, png_bytep row,
{
int a, b, c, pa, pb, pc, p;
+ if (prev_row-bpp < prev_row_start) {
+ fprintf(stderr, "MULTIBYTE PREV @%p<%p\n",
+ prev_row-bpp, prev_row_start);
+ abort();
+ }
+ if (row-bpp < row_start) {
+ fprintf(stderr, "MULTIBYTE ROW @%p<%p\n", row-bpp, row_start);
+ abort();
+ }
+
c = *(prev_row - bpp);
a = *(row - bpp);
b = *prev_row++;
If what you say is true the test will actually abort() If it does I'll send you a patch to write out the test case PNG file to /tmp, then you can demo the bug IRL, we have a useful test case and we can fix it.
HI @jbowler , so I have tried the patch but I suspect there is another possible bug reported by the fuzzer prior the main if so I have adapted it a bit putting it at the beginning
static void
png_read_filter_row_paeth_multibyte_pixel(png_row_infop row_info, png_bytep row,
png_const_bytep prev_row)
{
const png_const_bytep prev_row_start = prev_row;
const png_const_bytep row_start = row;
unsigned int bpp = (row_info->pixel_depth + 7) >> 3;
png_bytep rp_end = row + bpp;
printf("bpp: %d\n", bpp);
printf("prev_row: %p\n");
printf("prev_row_start: %p\n", prev_row_start);
printf("condition: %d\n", prev_row-bpp < prev_row_start);
if (prev_row-bpp < prev_row_start) {
fprintf(stderr, "MULTIBYTE PREV @%p<%p\n",
prev_row-bpp, prev_row_start);
abort();
}
No when I run the reproducer I can hit the abort:
➜ oss-fuzz git:(master) ✗ sudo python3 infra/helper.py reproduce libpng libpng_read_fuzzer ../oss/issue424/reproducer
INFO:root:Running: docker run --rm --privileged --platform linux/amd64 -i -v /home/alberto/progetti/oss-fuzz/build/out/libpng:/out -v /home/alberto/progetti/oss/issue424/reproducer:/testcase -t gcr.io/oss-fuzz-base/base-runner reproduce libpng_read_fuzzer -runs=100.
+ FUZZER=libpng_read_fuzzer
+ shift
+ '[' '!' -v TESTCASE ']'
+ TESTCASE=/testcase
+ '[' '!' -f /testcase ']'
+ export RUN_FUZZER_MODE=interactive
+ RUN_FUZZER_MODE=interactive
+ export FUZZING_ENGINE=libfuzzer
+ FUZZING_ENGINE=libfuzzer
+ export SKIP_SEED_CORPUS=1
+ SKIP_SEED_CORPUS=1
+ run_fuzzer libpng_read_fuzzer -runs=100 /testcase
/out/libpng_read_fuzzer -rss_limit_mb=2560 -timeout=25 -runs=100 /testcase -dict=png.dict < /dev/null
Dictionary: 28 entries
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3447305415
INFO: Loaded 1 modules (5502 inline 8-bit counters): 5502 [0x6f7db0, 0x6f932e),
INFO: Loaded 1 PC tables (5502 PCs): 5502 [0x691380,0x6a6b60),
/out/libpng_read_fuzzer: Running 1 inputs 100 time(s) each.
Running: /testcase
bpp: 2
prev_row: 0x706000000080
prev_row_start: 0x706000000080
condition: 1
MULTIBYTE PREV @0x70600000007e<0x706000000080
==17== ERROR: libFuzzer: deadly signal
#0 0x449b39 in __sanitizer_print_stack_trace /src/llvm-project/compiler-rt/lib/msan/msan.cpp:735:3
#1 0x4e1008 in fuzzer::PrintStackTrace() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerUtil.cpp:210:5
#2 0x4c5ce3 in fuzzer::Fuzzer::CrashCallback() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:233:3
#3 0x496f18 in SignalAction(int, void*, void*) /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:998:3
#4 0x7f4e635a741f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1441f) (BuildId: 7b4536f41cdaa5888408e82d0836e33dcf436466)
#5 0x7f4e633b700a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300a) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#6 0x7f4e63396858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x22858) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#7 0x5d03e7 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c
#8 0x5ca2ad in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4168:7
#9 0x568b11 in OSS_FUZZ_png_read_row /src/libpng/pngread.c:548:10
#10 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#11 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#12 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#13 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#14 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#15 0x7f4e63398082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#16 0x41f62d in _start (/out/libpng_read_fuzzer+0x41f62d)
DEDUP_TOKEN: __sanitizer_print_stack_trace--fuzzer::PrintStackTrace()--fuzzer::Fuzzer::CrashCallback()
NOTE: libFuzzer has rudimentary signal handlers.
Combine libFuzzer with AddressSanitizer or similar for better crash reports.
SUMMARY: libFuzzer: deadly signal
➜ oss-fuzz git:(master) ✗
EDIT:
We have already the png that we can use:
➜ oss-fuzz git:(master) ✗ file ../oss/issue424/reproducer
../oss/issue424/reproducer: PNG image data, 2 x 130, 8-bit gray+alpha, non-interlaced
➜ oss-fuzz git:(master) ✗
I think we have proved the bug so now we can work on the patch. What do you think?
Thanks
Ok; please attach that file to the bug report and please also attach the git commit you used and the changes you actually made; this wasn't in the patch I sent:
bpp: 2
prev_row: 0x706000000080
prev_row_start: 0x706000000080
condition: 1
Hi, i used a public repo under my name.
Please use this zip in cases you wanted the reproduce the bug: repro.zip
Do you want to setup the fuzzer as well?
Yes, your change will always fail; you're just doing this test:
if (pointer - 2 < pointer)
Surely it is obvious that this test is always true?
So I don't understand at this point; you are clearly saying that there is no problem in the second while loop, prev_row-bpp never points at something that has not already been read in the same function. But does the fuzzer fail with my patch (i.e. fail without my abort()).
Your test image is only 1 pixel wide; the second while loop is never entered. So does the fuzzer fail on that specific image?
Hi @jbowler , I made a mistake yes, thanks for noticing it. I have spent more time on the issue and I think I have identified the problem now.
The bug is reported because the png_ptr->row_buf is considered to be not initialised. In fact if I added a very simple memset(png_ptr->row_ptr, 0xff, 5); right before the call to png_read_IDAT_data inside png_read_IDAT_data. I have investigated a bit what the png_read_IDAT_data does and I think it uses PNG_INFLATE for inflating the new data inside png_ptr->row_buf. In order to make sure of it I have added a couple of printf so I could verify it.
An example output is now the following
png_ptr->big_row_buf: 0x706000000000
png_ptr->big_prev_row: 0x706000000060
png_ptr->row_buf : 0x70600000001f
png_ptr->prev_row: 0x70600000007f
memset png_ptr->prev_row for 5 bytes. [ 0x70600000007f - 0x706000000084 ]
0: 255, 1: 120 2: 48 3: 97 4: 34
before png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
png_ptr->zstream.avail_in == 0
output != NULL
png_ptr->zstream.avail_out set to 5
before PNG_INFLATE 0: 255 1: 120 2: 48 3: 97 4: 34
after PNG_INFLATE 0: 4 1: 4 2: 4 3: 4 4: 4
after png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
0: 4, 1: 4 2: 4 3: 4 4: 4
bpp: 2
Processing the first pixel
Increasing row
Increasing row
Remainder
while loop
row: 0x706000000022
rp_end: 0x706000000024
row: 0x706000000022
row_start: 0x706000000020
c = *(0x706000000082 - 2)
a = *(0x706000000022 - 2)
b = *(0x706000000083)
p = b - c
pc = a - c
Setting pa
Setting pb
Setting pc
pb < pa ?
==17==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5d125c in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4112:11
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:552:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7fe02026b082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#9 0x41f62d in _start (/out/libpng_read_fuzzer+0x41f62d)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d06e4 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:552:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7fe02026b082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d06e4 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:552:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7fe02026b082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was created by a heap allocation
#0 0x453ef0 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:895:3
#1 0x4a6401 in limited_malloc(png_struct_def*, unsigned long) /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:89:10
#2 0x56530f in OSS_FUZZ_png_malloc_base /src/libpng/pngmem.c:91:17
#3 0x56530f in OSS_FUZZ_png_malloc /src/libpng/pngmem.c:179:10
#4 0x5cdeb0 in OSS_FUZZ_png_read_start_row /src/libpng/pngrutil.c:4647:44
#5 0x5681e2 in OSS_FUZZ_png_read_update_info /src/libpng/pngread.c:275:10
#6 0x4a7b4d in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:191:3
#7 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#8 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#9 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#10 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#11 0x7fe02026b082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: __interceptor_malloc--limited_malloc(png_struct_def*, unsigned long)--OSS_FUZZ_png_malloc_base
SUMMARY: MemorySanitizer: use-of-uninitialized-value /src/libpng/pngrutil.c:4112:11 in png_read_filter_row_paeth_multibyte_pixel
Unique heap origins: 58
Stack depot allocated bytes: 9764880
Unique origin histories: 29
History depot allocated bytes: 196608
Exiting
I haven't spent some time reading the zlib source code but since the values changed I 'd consider the png_ptr->row_buf to be initialised.
I'm inclined to say that this is an invalid bug. Do you think it would be a good practice to memset the png_ptr->row_buf anyway?
All the code is available at this repo.
I have also attached again the reproducer in case I made any mistake
➜ oss-fuzz git:(master) ✗ md5sum ../oss/issue424/reproducer
3ae4407f8c793d5c3c8e66cb82cea242 ../oss/issue424/reproducer
➜ oss-fuzz git:(master) ✗
What's your opinion?
Thanks
@thealberto is it reasonable to assume this is a bug in the memory sanitizer?
Hi @ctruta ,
I quickly had a look inside the inflate function and the png_ptr->row_buf is never used but at after the PNG_INFLATE is called the content inside png_ptr->row_buf is different as shown below:
➜ oss-fuzz git:(master) ✗ sudo python3 infra/helper.py reproduce libpng libpng_read_fuzzer ../oss/issue424/reproducer2
INFO:root:Running: docker run --rm --privileged --platform linux/amd64 -i -v /home/alberto/progetti/oss-fuzz/build/out/libpng:/out -v /home/alberto/progetti/oss/issue424/reproducer2:/testcase -t gcr.io/oss-fuzz-base/base-runner reproduce libpng_read_fuzzer -runs=100.
+ FUZZER=libpng_read_fuzzer
+ shift
+ '[' '!' -v TESTCASE ']'
+ TESTCASE=/testcase
+ '[' '!' -f /testcase ']'
+ export RUN_FUZZER_MODE=interactive
+ RUN_FUZZER_MODE=interactive
+ export FUZZING_ENGINE=libfuzzer
+ FUZZING_ENGINE=libfuzzer
+ export SKIP_SEED_CORPUS=1
+ SKIP_SEED_CORPUS=1
+ run_fuzzer libpng_read_fuzzer -runs=100 /testcase
/out/libpng_read_fuzzer -rss_limit_mb=2560 -timeout=25 -runs=100 /testcase -dict=png.dict < /dev/null
Dictionary: 28 entries
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3331230496
INFO: Loaded 1 modules (5503 inline 8-bit counters): 5503 [0x6f8db0, 0x6fa32f),
INFO: Loaded 1 PC tables (5503 PCs): 5503 [0x6925f0,0x6a7de0),
/out/libpng_read_fuzzer: Running 1 inputs 100 time(s) each.
Running: /testcase
png_ptr->big_row_buf: 0x706000000000
png_ptr->big_prev_row: 0x706000000060
png_ptr->row_buf : 0x70600000001f
png_ptr->prev_row: 0x70600000007f
memset png_ptr->prev_row for 5 bytes. [ 0x70600000007f - 0x706000000084 ]
0: 255, 1: 255 2: 255 3: 255 4: 255
before png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
png_ptr->zstream.avail_in == 0
output != NULL
png_ptr->zstream.avail_out set to 5
before PNG_INFLATE 0: 255 1: 255 2: 255 3: 255 4: 255 <<== here ( values for the first 5 bytes inside png_ptr->row_buf )
after PNG_INFLATE 0: 4 1: 4 2: 4 3: 4 4: 4 <<== here
Do you understand where png_ptr->row_buf is modified? What are all those 4?
The values are different so they seems to be initialised but I don't understand how
Thanks
I think I now understand:
#ifdef PNG_SEQUENTIAL_READ_SUPPORTED
void /* PRIVATE */
png_read_IDAT_data(png_structrp png_ptr, png_bytep output,
png_alloc_size_t avail_out)
{
/* Loop reading IDATs and decompressing the result into output[avail_out] */
png_ptr->zstream.next_out = output; <<== our png_ptr->row_buf
png_ptr->zstream.avail_out = 0; /* safety: set below */
So the zstream struct has access to the png_ptr->row_buf... I'll try to understand the code of the inflate function which part of zlib and see what it does.
Is it OK?
Thanks
Oh. That's completely different. Please attach the actual failing data stream (i.e. input file, or whatever it is) to this bug report.
BTW: unless you have used the z_stream API of zlib before it is, while very simple and powerful, not actually easy to understand. Indeed someone may have messed up the end conditions.
HI, have you tried to reproduce the case already with the fuzzer? I have just used the files that I have attached a couple of messages ago.
Any I have attached them again for clarity.
Let me know how it goes.
Thanks
Oh; I hadn't realized that those are PNG files. I will have a look at them.
Nope: no bug that I can see. pngfix, pngcp both handle the files correctly (they error out before they even attempt to call zlib). libpng_read_fuzzer actually manages to read a complete row from the file - it is ignoring the CRC error. pngcp isn't and pngfix attempts to read past it but correctly finds a damaged zlib stream; it is of length 6 and apparently consists of just "4" repeated when decoded. It seems to be truncated after four rows, from pngcheck -f -vv
The fuzzer hits png_read_IDAT_data four times before it exits with a normal error code (I had to hack main(), it isn't included with the source code); so the fuzzer is doing what pngcheck does with -f.
At the point where the fuzzer fails zlib has plonked another 3 \004 bytes into the output buffer but there are still two to go (the row is 5 bytes long, including the filter byte). The IDAT chunk is ended so we hit the png_crc_finish at my line 4172 of pngrutil.c. There is no more data so the code flow goes back into the fuzzer read function, user_read_data on line 70 where buf_state->bytes_left is 0 (i.e. end of file; right at the CRC) and this correctly calls png_error("read error") at line 73.
Most of the code apparently in png_error is skipped because STDIO was disabled in the fuzzer build; not a typical arrangement but likely in IoT devices so png_default_error is called which calls longjmp.
This takes us back to pngmem.c, png_free; actually line 152 of the fuzzer, though gdb will give another line.
NOT a bug anywhere.
There might be one problem in the fuzzer; it is C++ and all of the code in there apart from the equivalent of main() is C++, yet it is written in C. The problem is that a longjmp is going to invoke all the cleanup code that is in there, because the whole shebang is compiled as a C++ program.
The rule for libpng programming, because it uses longjmp, is that no C++ functions can be used in the callbacks, but "limited_malloc" and "default_free" are both C++ functions. I don't quite know how g++ deals with this; it's bogus but produces no error on compilation but this definitely needs to be fixed.
My impression is that the whole block of code is C, so it should be identified as such. Whether it's the original issue is a different matter; the maintainer of the fuzzer needs to comment.
Hi @jbowler,
I have run some additional test because I wanted to see if the png_crc_finish function was called and I don't think it is ever called. The updated code is available at the usual repo.
Please look at the run of the fuzzer with reproducer and reproducer2
➜ oss-fuzz git:(master) ✗ sudo python3 infra/helper.py reproduce libpng libpng_read_fuzzer ../oss/issue424/reproducer
INFO:root:Running: docker run --rm --privileged --platform linux/amd64 -i -v /home/alberto/progetti/oss-fuzz/build/out/libpng:/out -v /home/alberto/progetti/oss/issue424/reproducer:/testcase -t gcr.io/oss-fuzz-base/base-runner reproduce libpng_read_fuzzer -runs=100.
+ FUZZER=libpng_read_fuzzer
+ shift
+ '[' '!' -v TESTCASE ']'
+ TESTCASE=/testcase
+ '[' '!' -f /testcase ']'
+ export RUN_FUZZER_MODE=interactive
+ RUN_FUZZER_MODE=interactive
+ export FUZZING_ENGINE=libfuzzer
+ FUZZING_ENGINE=libfuzzer
+ export SKIP_SEED_CORPUS=1
+ SKIP_SEED_CORPUS=1
+ run_fuzzer libpng_read_fuzzer -runs=100 /testcase
/out/libpng_read_fuzzer -rss_limit_mb=2560 -timeout=25 -runs=100 /testcase -dict=png.dict < /dev/null
Dictionary: 28 entries
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3631256167
INFO: Loaded 1 modules (5503 inline 8-bit counters): 5503 [0x6f8db0, 0x6fa32f),
INFO: Loaded 1 PC tables (5503 PCs): 5503 [0x692620,0x6a7e10),
/out/libpng_read_fuzzer: Running 1 inputs 100 time(s) each.
Running: /testcase
png_ptr->big_row_buf: 0x706000000000
png_ptr->big_prev_row: 0x706000000060
png_ptr->row_buf : 0x70600000001f
png_ptr->prev_row: 0x70600000007f
memset png_ptr->prev_row for 5 bytes. [ 0x70600000007f - 0x706000000084 ]
0: 255, 1: 120 2: 48 3: 97 4: 34
before png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
png_ptr->zstream.avail_in == 0
png_ptr->idat_size is 6
output != NULL
png_ptr->zstream.avail_out set to 5
before PNG_INFLATE 0: 255 1: 120 2: 48 3: 97 4: 34
after PNG_INFLATE 0: 4 1: 4 2: 4 3: 4 4: 4
after png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
0: 4, 1: 4 2: 4 3: 4 4: 4
bpp: 2
Processing the first pixel
Increasing row
Increasing row
Remainder
while loop
row: 0x706000000022
rp_end: 0x706000000024
row: 0x706000000022
row_start: 0x706000000020
c = *(0x706000000082 - 2)
a = *(0x706000000022 - 2)
b = *(0x706000000083)
p = b - c
pc = a - c
Setting pa
Setting pb
Setting pc
pb < pa ?
==17==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5d12cc in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4112:11
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:553:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7f637a058082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#9 0x41f62d in _start (/out/libpng_read_fuzzer+0x41f62d)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d0754 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:553:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7f637a058082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d0754 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:553:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7f637a058082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was created by a heap allocation
#0 0x453ef0 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:895:3
#1 0x4a6401 in limited_malloc(png_struct_def*, unsigned long) /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:89:10
#2 0x56530f in OSS_FUZZ_png_malloc_base /src/libpng/pngmem.c:91:17
#3 0x56530f in OSS_FUZZ_png_malloc /src/libpng/pngmem.c:179:10
#4 0x5cdf20 in OSS_FUZZ_png_read_start_row /src/libpng/pngrutil.c:4649:44
#5 0x5681e2 in OSS_FUZZ_png_read_update_info /src/libpng/pngread.c:275:10
#6 0x4a7b4d in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:191:3
#7 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#8 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#9 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#10 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#11 0x7f637a058082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: __interceptor_malloc--limited_malloc(png_struct_def*, unsigned long)--OSS_FUZZ_png_malloc_base
SUMMARY: MemorySanitizer: use-of-uninitialized-value /src/libpng/pngrutil.c:4112:11 in png_read_filter_row_paeth_multibyte_pixel
Unique heap origins: 58
Stack depot allocated bytes: 9764880
Unique origin histories: 29
History depot allocated bytes: 196608
Exiting
➜ oss-fuzz git:(master) ✗ sudo python3 infra/helper.py reproduce libpng libpng_read_fuzzer ../oss/issue424/reproducer2
INFO:root:Running: docker run --rm --privileged --platform linux/amd64 -i -v /home/alberto/progetti/oss-fuzz/build/out/libpng:/out -v /home/alberto/progetti/oss/issue424/reproducer2:/testcase -t gcr.io/oss-fuzz-base/base-runner reproduce libpng_read_fuzzer -runs=100.
+ FUZZER=libpng_read_fuzzer
+ shift
+ '[' '!' -v TESTCASE ']'
+ TESTCASE=/testcase
+ '[' '!' -f /testcase ']'
+ export RUN_FUZZER_MODE=interactive
+ RUN_FUZZER_MODE=interactive
+ export FUZZING_ENGINE=libfuzzer
+ FUZZING_ENGINE=libfuzzer
+ export SKIP_SEED_CORPUS=1
+ SKIP_SEED_CORPUS=1
+ run_fuzzer libpng_read_fuzzer -runs=100 /testcase
/out/libpng_read_fuzzer -rss_limit_mb=2560 -timeout=25 -runs=100 /testcase -dict=png.dict < /dev/null
Dictionary: 28 entries
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3634970270
INFO: Loaded 1 modules (5503 inline 8-bit counters): 5503 [0x6f8db0, 0x6fa32f),
INFO: Loaded 1 PC tables (5503 PCs): 5503 [0x692620,0x6a7e10),
/out/libpng_read_fuzzer: Running 1 inputs 100 time(s) each.
Running: /testcase
png_ptr->big_row_buf: 0x706000000000
png_ptr->big_prev_row: 0x706000000060
png_ptr->row_buf : 0x70600000001f
png_ptr->prev_row: 0x70600000007f
memset png_ptr->prev_row for 5 bytes. [ 0x70600000007f - 0x706000000084 ]
0: 255, 1: 120 2: 48 3: 97 4: 34
before png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
png_ptr->zstream.avail_in == 0
png_ptr->idat_size is 6
output != NULL
png_ptr->zstream.avail_out set to 5
before PNG_INFLATE 0: 255 1: 120 2: 48 3: 97 4: 34
after PNG_INFLATE 0: 4 1: 4 2: 4 3: 4 4: 4
after png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
0: 4, 1: 4 2: 4 3: 4 4: 4
bpp: 2
Processing the first pixel
Increasing row
Increasing row
Remainder
while loop
row: 0x706000000022
rp_end: 0x706000000024
row: 0x706000000022
row_start: 0x706000000020
c = *(0x706000000082 - 2)
a = *(0x706000000022 - 2)
b = *(0x706000000083)
p = b - c
pc = a - c
Setting pa
Setting pb
Setting pc
pb < pa ?
==17==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5d12cc in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4112:11
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:553:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7f69fa86d082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#9 0x41f62d in _start (/out/libpng_read_fuzzer+0x41f62d)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d0754 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:553:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7f69fa86d082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d0754 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
#1 0x5ca69d in OSS_FUZZ_png_read_filter_row /src/libpng/pngrutil.c:4174:7
#2 0x568edc in OSS_FUZZ_png_read_row /src/libpng/pngread.c:553:10
#3 0x4a7d50 in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:199:7
#4 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#6 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#7 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#8 0x7f69fa86d082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was created by a heap allocation
#0 0x453ef0 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:895:3
#1 0x4a6401 in limited_malloc(png_struct_def*, unsigned long) /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:89:10
#2 0x56530f in OSS_FUZZ_png_malloc_base /src/libpng/pngmem.c:91:17
#3 0x56530f in OSS_FUZZ_png_malloc /src/libpng/pngmem.c:179:10
#4 0x5cdf20 in OSS_FUZZ_png_read_start_row /src/libpng/pngrutil.c:4649:44
#5 0x5681e2 in OSS_FUZZ_png_read_update_info /src/libpng/pngread.c:275:10
#6 0x4a7b4d in LLVMFuzzerTestOneInput /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:191:3
#7 0x4c7283 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#8 0x4b29e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#9 0x4b828c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#10 0x4e17c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#11 0x7f69fa86d082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
DEDUP_TOKEN: __interceptor_malloc--limited_malloc(png_struct_def*, unsigned long)--OSS_FUZZ_png_malloc_base
SUMMARY: MemorySanitizer: use-of-uninitialized-value /src/libpng/pngrutil.c:4112:11 in png_read_filter_row_paeth_multibyte_pixel
Unique heap origins: 58
Stack depot allocated bytes: 9764880
Unique origin histories: 29
History depot allocated bytes: 196608
Exiting
➜ oss-fuzz git:(master) ✗
So the printout shows that the pointer values are correct, except that the first of this pair of lines in your code is clearly wrong:
printf("b = *(%p)\n", prev_row + 1);
b = *prev_row++;
It's a POST increment; the actual value of "b" comes from 0x706000000082, same as the value in the earlier assignment to "a":
c = *(prev_row - bpp);
What is more we know that the first four bytes of prev_row were initialized, from the memset comment in the logs and by code examination; the two rows are 4 bytes long because the PNG has a width of 2 GA 8-bit per component pixels. So all the prev_row memory is completely initialized as this point and so are "a" and "c":
memset png_ptr->prev_row for 5 bytes. [ 0x70600000007f - 0x706000000084 ]
That's misleading too - the last byte initialized is actually 0x706000000083 and that will be read the next, and last, time round the second while loop. prev_row will end up at 0x706000000084 but it isn't accessed.
Anyway, that isn't what the fuzzer is complaining about; it says that the original store was actually in the first loop:
DEDUP_TOKEN: png_read_filter_row_paeth_multibyte_pixel--OSS_FUZZ_png_read_filter_row--OSS_FUZZ_png_read_row
Uninitialized value was stored to memory at
#0 0x5d0754 in png_read_filter_row_paeth_multibyte_pixel /src/libpng/pngrutil.c:4064:14
This is the loop:
4060: while (row < rp_end)
4061: {
4062: int a = *row + *prev_row++; // FROM HERE
4063: printf("\tIncreasing row\n");
4064: *row++ = (png_byte)a; // <<<< STORED HERE
4065: }
We know that prev_row[4] is completely initialized, so if the fuzzer were right it would have to be row[0] which is not, because in the second loop, again from the logging messages:
a = *(0x706000000022 - 2)
That's the load of the value written at 4064 and computed at 4062.
But then you proved that the byte had been written; adding emphasis to the second line:
after png_read_IDAT_data -> png_ptr->row_buf: 0x70600000001f
0: 4, 1: 4 2: 4 3: 4 4: 4
That's correct: we know the 6 byte IDAT expands to a row of 4's; run "pngcheck -vv -f" on either file.
So the report is just plain wrong; it's complaining that a value was uninitialized when we can clearly see it has been initialized. In fact I believe that happens inside inflate so maybe FuzzingEngine doesn't see it?
It might help to add a couple of scripts to contrib/libpng/oss-fuzz; the stuff there is just a set of files for docker. It is unusable without a knowledge of how to set up a docker and it's not clear that everything is there; I don't understand where -lFuzzingEngine comes from.
A second script to do the build on a regular machine would actually allow debugging; doing a ssh into a docker then having to install debuggers etc is a waste of time. There's no justification for doing this kind of stuff on a docker for those of us who don't have access to expensive docker accounts in the cloud.
It's also not clear that "build.sh" is using the correct zlib. It just uses "-lz" which seems wrong; libpng.git and zlib.git are both cloned in Dockerfile but only libpng is built. So there is a lot of missing stuff there; look at the final line in the header of build.sh!