libpng
libpng copied to clipboard
glennrp
An integer overflow happened in png_read_png->..->png_read_filter_row_avg
Hi, we found an iteger overflow happend in png_read_png by fuzzing and it evently caused a crash.
Similar to #456, let assume the read 13 bytes are : {0, 0, 0, 32, 0, 0, 0, 32, 8, 3, 0, 0, 0}. The following filed of png_ptr will be set to pixel_depth=0x8, row_info->rowbytes=0.
Then, the following codes in png_read_png->png_read_image->png_read_row->png_read_filter_row->png_read_filter_row_avg will occur an integer overflow.
3971 unsigned int bpp = (row_info->pixel_depth + 7) >> 3;
→ 3972 size_t istop = row_info->rowbytes - bpp;
Here, row_info->rowbytes=0 and bpp=1, and the istop=0xffffffffffffffff.
Here is the trigger case, you can compile it to reproduce the crash.
#include "png.h"
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
typedef uint8_t u8;
typedef uint64_t u64;
typedef int8_t i8;
typedef int32_t i32;
void GENERATED_hopper_callback_10(struct png_struct_def *arg0, void *arg1){
}
int main() {
i8 v0_tmp[] = {49, 46, 54, 46, 51, 55, 0, }; // user_png_ver
i8 *v0 = malloc(sizeof v0_tmp);
memcpy(v0, v0_tmp, sizeof v0_tmp);
i8 *v1 = v0; // user_png_ver
void *v2 = NULL; // error_ptr
void (*v3)(struct png_struct_def *,i8 *) = NULL; // error_fn
void (*v4)(struct png_struct_def *,i8 *) = NULL; // warn_fn
void *v5 = NULL; // mem_ptr
void * (*v6)(struct png_struct_def *,u64 ) = NULL; // malloc_fn
void (*v7)(struct png_struct_def *,void *) = NULL; // free_fn
struct png_struct_def *v8 = png_create_read_struct_2(v1, v2, v3, v4, v5, v6, v7); // png_ptr
if (v8 == NULL) return 0;
struct png_struct_def *v10 = v8; // png_ptr
u8 v11_tmp[] = {137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 32, 0, 0, 0, 32, 8, 3, 0, 0, 0, 68, 164, 138, 198, 0, 0, 0, 4, 103, 65, 77, 65, 0, 0, 156, 64, 32, 13, 228, 203, 0, 0, 0, 15, 80, 76, 84, 69, 102, 204, 204, 255, 255, 255, 0, 0, 0, 51, 153, 102, 153, 255, 204, 62, 76, 175, 21, 0, 0, 0, 25, 116, 69, 88, 116, 83, 111, 102, 116, 119, 97, 114, 101, 0, 65, 100, 111, 98, 101, 32, 73, 109, 97, 103, 101, 82, 101, 97, 100, 121, 113, 201, 101, 60, 0, 0, 0, 91, 73, 68, 65, 84, 56, 203, 221, 147, 65, 10, 128, 64, 12, 3, 51, 205, 254, 255, 205, 30, 84, 216, 69, 219, 122, 241, 160, 115, 205, 64, 32, 180, 6, 65, 16, 5, 32, 98, 228, 40, 16, 49, 148, 226, 159, 10, 0, 149, 112, 172, 155, 10, 128, 61, 27, 87, 193, 150, 192, 105, 197, 158, 23, 194, 89, 83, 8, 75, 126, 47, 76, 121, 34, 52, 75, 186, 17, 150, 33, 223, 169, 248, 236, 201, 201, 57, 241, 228, 121, 27, 54, 210, 102, 3, 53, 127, 203, 12, 109, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130, 0, }; // file_buf
u8 *v11 = malloc(sizeof v11_tmp);
memcpy(v11, v11_tmp, sizeof v11_tmp);
char* v12 = "file___filename_11"; // __filename
FILE *f_v12 = fopen(v12, "wb");
fwrite(v11, sizeof v11_tmp, 1, f_v12);
fclose(f_v12);
i8 v13_tmp[] = {114, 98, 43, 0, }; // __modes
i8 *v13 = malloc(sizeof v13_tmp);
memcpy(v13, v13_tmp, sizeof v13_tmp);
i8 *v14 = v13; // __modes
struct _IO_FILE *v15 = fopen(v12, v14); // fp
if (v15 == NULL) return 0;
struct _IO_FILE *v17 = v15; // fp
png_init_io(v10, v17); // $relative
struct png_info_def *v19 = png_create_info_struct(v10); // info_ptr
if (v19 == NULL) return 0;
struct png_info_def *v21 = v19; // info_ptr
i32 v22 = -1; // transforms
void *v23 = NULL; // params
i32 v24 = -626394128; // allowed
i8 *v25 = NULL; // mem_ptr
void * (*v26)(struct png_struct_def *,u64 ) = NULL; // malloc_fn
void (*v27)(struct png_struct_def *,void *) = &GENERATED_hopper_callback_10; // free_fn
png_set_mem_fn(v10, v25, v26, v27); // $relative
png_set_benign_errors(v10, v24); // $relative
png_read_update_info(v10, v21); // $relative
png_read_png(v10, v21, v22, v23); // $relative
png_read_info(v10, v21); // $target
}
Hmm, I don't understand why you observed row_info->rowbytes == 0. The 13 IHDR bytes you gave say that width = 32, bit depth = 8, color type = 3 (indexed color). Therefore, rowbytes should be 32.
Also, because width and bit depth are always positive, there is no valid image where rowbytes = 0.
Did your fuzz case forget to call some mandatory libpng functions or call them in the wrong order? (Sorry, I'm not familiar with the codebase.)
@nayuki ,Sorry about somethings not described clearly.
row_info->rowbytes == 0 is because row_info is computed from png_ptr->pixel_depth and png_ptr->iwidth, where png_ptr->pixel_depth=8 is read from IHDR and png_ptr->iwidth holds the default value 0.
382 void PNGAPI
383 png_read_row(png_structrp png_ptr, png_bytep row, png_bytep dsp_row)
384 {
...
400 row_info.width = png_ptr->iwidth; /* NOTE: width of current interlaced row */
401 row_info.color_type = png_ptr->color_type;
402 row_info.bit_depth = png_ptr->bit_depth;
403 row_info.channels = png_ptr->channels;
404 row_info.pixel_depth = png_ptr->pixel_depth;
→ 405 row_info.rowbytes = PNG_ROWBYTES(row_info.pixel_depth, row_info.width);
Okay, I was able to reproduce your bug. I still don't know libpng well enough to tell if the sequence of function calls is wrong.
But anyway, I manually minimized your test program:
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include "png.h"
void free_fn(struct png_struct_def *a, void *b){}
int main() {
const char *tempfilename = "fuzz.png";
{
uint8_t pngbytes[] = {
137, 'P', 'N', 'G', '\r', '\n', 26, '\n',
0, 0, 0, 13, 'I','H','D','R', 0, 0, 0, 32, 0, 0, 0, 32, 8, 3, 0, 0, 0, 68, 164, 138, 198,
0, 0, 0, 4, 'g','A','M','A', 0, 0, 156, 64, 32, 13, 228, 203,
0, 0, 0, 15, 'P','L','T','E', 102, 204, 204, 255, 255, 255, 0, 0, 0, 51, 153, 102, 153, 255, 204, 62, 76, 175, 21,
0, 0, 0, 25, 't','E','X','t', 83, 111, 102, 116, 119, 97, 114, 101, 0, 65, 100, 111, 98, 101, 32, 73, 109, 97, 103, 101, 82, 101, 97, 100, 121, 113, 201, 101, 60,
0, 0, 0, 91, 'I','D','A','T', 56, 203, 221, 147, 65, 10, 128, 64, 12, 3, 51, 205, 254, 255, 205, 30, 84, 216, 69, 219, 122, 241, 160, 115, 205, 64, 32, 180, 6, 65, 16, 5, 32, 98, 228, 40, 16, 49, 148, 226, 159, 10, 0, 149, 112, 172, 155, 10, 128, 61, 27, 87, 193, 150, 192, 105, 197, 158, 23, 194, 89, 83, 8, 75, 126, 47, 76, 121, 34, 52, 75, 186, 17, 150, 33, 223, 169, 248, 236, 201, 201, 57, 241, 228, 121, 27, 54, 210, 102, 3, 53, 127, 203, 12, 109,
0, 0, 0, 0, 'I','E','N','D', 174, 66, 96, 130,
};
FILE *outfile = fopen(tempfilename, "wb");
fwrite(pngbytes, sizeof(pngbytes), 1, outfile);
fclose(outfile);
}
FILE *infile = fopen(tempfilename, "rb+");
if (infile == NULL) return 0;
struct png_struct_def *png_ptr = png_create_read_struct_2("1.6.37", NULL, NULL, NULL, NULL, NULL, NULL);
if (png_ptr == NULL) return 0;
png_init_io(png_ptr, infile);
struct png_info_def *info_ptr = png_create_info_struct(png_ptr);
if (info_ptr == NULL) return 0;
png_set_mem_fn(png_ptr, NULL, NULL, free_fn);
png_set_benign_errors(png_ptr, -626394128);
png_read_update_info(png_ptr, info_ptr);
png_read_png(png_ptr, info_ptr, -1, NULL);
png_read_info(png_ptr, info_ptr);
}
Then I compiled the code with sanitizers, and ran it to observe an out-of-bounds read:
user@ubuntu:~/libpng$ clang -Wall fuzz.c png.c pngerror.c pngget.c pngmem.c pngpread.c pngread.c pngrio.c pngrtran.c pngrutil.c pngset.c pngtrans.c pngwio.c pngwrite.c pngwtran.c pngwutil.c crc32.c deflate.c inflate.c inftrees.c adler32.c trees.c zutil.c inffast.c -lm -fsanitize=undefined,address -g
user@ubuntu:~/libpng$ ./a.out
libpng warning: gAMA: gamma value does not match libpng estimate
libpng warning: invalid after png_start_read_image or png_read_update_info
libpng warning: invalid after png_start_read_image or png_read_update_info
libpng warning: invalid after png_start_read_image or png_read_update_info
libpng warning: invalid after png_start_read_image or png_read_update_info
libpng warning: invalid after png_start_read_image or png_read_update_info
libpng warning: invalid after png_start_read_image or png_read_update_info
libpng warning: png_read_update_info/png_start_read_image: duplicate call
=================================================================
==6255==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x606000000051 at pc 0x00000061ca2c bp 0x7ffe963b2f60 sp 0x7ffe963b2f58
READ of size 1 at 0x606000000051 thread T0
#0 0x61ca2b in png_read_filter_row_avg ~/libpng/pngrutil.c:3985:31
#1 0x61098b in png_read_filter_row ~/libpng/pngrutil.c:4144:7
#2 0x5300cb in png_read_row ~/libpng/pngread.c:548:10
#3 0x533c79 in png_read_image ~/libpng/pngread.c:753:10
#4 0x5386fd in png_read_png ~/libpng/pngread.c:1241:4
#5 0x4c62f1 in main ~/libpng/fuzz.c:39:5
#6 0x7f7824befcb1 in __libc_start_main csu/../csu/libc-start.c:314:16
#7 0x41c32d in _start (~/libpng/a.out+0x41c32d)
0x606000000051 is located 0 bytes to the right of 49-byte region [0x606000000020,0x606000000051)
allocated by thread T0 here:
#0 0x49639d in malloc (~/libpng/a.out+0x49639d)
#1 0x511c98 in png_malloc_base ~/libpng/pngmem.c:95:17
#2 0x511a7b in png_malloc ~/libpng/pngmem.c:179:10
#3 0x61920b in png_read_start_row ~/libpng/pngrutil.c:4611:44
#4 0x52d4df in png_read_update_info ~/libpng/pngread.c:275:10
#5 0x4c62db in main ~/libpng/fuzz.c:38:5
#6 0x7f7824befcb1 in __libc_start_main csu/../csu/libc-start.c:314:16
SUMMARY: AddressSanitizer: heap-buffer-overflow ~/libpng/pngrutil.c:3985:31 in png_read_filter_row_avg
Shadow bytes around the buggy address:
0x0c0c7fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c0c7fff8000: fa fa fa fa 00 00 00 00 00 00[01]fa fa fa fa fa
0x0c0c7fff8010: 00 00 00 00 00 00 01 fa fa fa fa fa fa fa fa fa
0x0c0c7fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0c7fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0c7fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0c7fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==6255==ABORTING
This is an app bug, not a libpng png.
png_read_update_info can only be called once and must not be called with png_read_png because png_read_png has to call it after setting the transforms. There are lots of png_app_error calls telling the app developer that the app is bad. By default these call png_error but png_set_benign_errors has been called and this turns the png_error off; this is probably a mistake because the error is, in this case, terminal.
Application errors are not benign. The relevant code line is line 1686 of pngset.c in the implementation of png_set_benign_errors. I suggest removing PNG_FLAG_APP_ERRORS_WARN in the future but that might actually cause apps with duplicate calls (which are safe so long as nothing relevant is changed between the two calls) to stop being able to read PNGs until the app is fixed so the change should be delayed until a future major release.
In this specific case pngstruct::transformations ends up set to 0xa0021, which is:
PNG_BGR
PNG_INVERT_MONO
PNG_GAMMA
PNG_FILLER
BGR, INVERT_MONO and GAMMA should not cause a problem; they don't alter the size of the output. BGR and INVERT_MONO are in pngtran.c (they are both read and write). The png_rtran_ok() check was not added to png_set_ APIs in pngtran.c because the check function is local to pngrtran.c. This is worth fixing.
This patch adds app_error or app_warning to the pngtrans.c function. app_warning is used in cases where I believe the application error won't result in miscalculation of the row sizes. It doesn't stop the crash in the C code above; it looks like info_ptr->row_pointers[] is uninitialized. 457.patch
In fact png_read_start_row has been called (from png_read_update_info) before the IHDR has been read. This is why pngstruct::iwidth is 0 (because at this point pngstruct::width is 0). This patch handles the case by calling png_error.
With this patch a png_error happens on png_read_start_row if the IHDR has not been read and the above code will output the message then call abort() (because no error handling has been set up).
@ctruta this is an app programming error, the patch detects it and is minimal but this is unnecessary for libpng 1.6 so should go into libpng 1.8