Skip to content

Commit 76b491d

Browse files
ctrlcctrlvsaitoha
authored andcommitted
[SECURITY] Avoid free'ing a wild pointer on PNG decode
In certain cases, a PNG could be fed into `load_png` which would act as a DoS vector. I fixed this in two ways: * making sure `rows` is instantiated to NULL and checking if it's NULL before freeing it; * the minimum length of PNG data is known to be 67 bytes. So, if it's less, we know we can error out. Resolves CVE-2020-11721. Closes saitoha/libpixel#134. Closes #9.
1 parent 3885ab8 commit 76b491d

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

src/loader.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ load_png(unsigned char /* out */ **result,
317317
# pragma GCC diagnostic push
318318
# pragma GCC diagnostic ignored "-Wclobbered"
319319
#endif
320-
unsigned char **rows;
320+
unsigned char **rows = NULL;
321321
png_color *png_palette = NULL;
322322
png_color_16 background;
323323
png_color_16p default_background;
@@ -336,7 +336,6 @@ load_png(unsigned char /* out */ **result,
336336
#endif /* HAVE_SETJMP && HAVE_LONGJMP */
337337

338338
status = SIXEL_FALSE;
339-
rows = NULL;
340339
*result = NULL;
341340

342341
png_ptr = png_create_read_struct(
@@ -348,6 +347,14 @@ load_png(unsigned char /* out */ **result,
348347
goto cleanup;
349348
}
350349

350+
// The minimum valid PNG is 67 bytes.
351+
// https://garethrees.org/2007/11/14/pngcrush/
352+
if (size < 67) {
353+
sixel_helper_set_additional_message("PNG data too small to be valid!");
354+
status = SIXEL_PNG_ERROR;
355+
goto cleanup;
356+
}
357+
351358
#if HAVE_SETJMP
352359
if (setjmp(png_jmpbuf(png_ptr)) != 0) {
353360
sixel_allocator_free(allocator, *result);
@@ -357,6 +364,7 @@ load_png(unsigned char /* out */ **result,
357364
}
358365
#endif /* HAVE_SETJMP */
359366

367+
360368
info_ptr = png_create_info_struct(png_ptr);
361369
if (!info_ptr) {
362370
sixel_helper_set_additional_message(
@@ -647,7 +655,10 @@ load_png(unsigned char /* out */ **result,
647655

648656
cleanup:
649657
png_destroy_read_struct(&png_ptr, &info_ptr,(png_infopp)0);
650-
sixel_allocator_free(allocator, rows);
658+
659+
if (rows != NULL) {
660+
sixel_allocator_free(allocator, rows);
661+
}
651662

652663
return status;
653664
}

0 commit comments

Comments
 (0)