[Png-mng-security] potentially serious memory handling error in libpng

Wed Feb 4 12:45:57 UTC 2009

----- Original Message ----- 
From: "Tavis Ormandy" <taviso at sdf.lonestar.org> 
To: glennrp at comcast.net, png-mng-security at simple.dallas.tx.us 
Cc: scarybeasts at gmail.com 
Sent: Tuesday, February 3, 2009 10:41:18 PM GMT -05:00 US/Canada Eastern 
Subject: potentially serious memory handling error in libpng 

Hey Glenn, Greg and others, 

I've discovered a potentially serious error in libpng, it requires an 
allocation failure at a specific time in order to work, and so probably 
explains why nobody has reported running into this by accident. 
Unfortunately, an attacker can usually create this easily (e.g. just 
preparing a highly compressable image of ridiculous size). 

The problem is in pngread.c, 

1433 if (info_ptr->row_pointers == NULL) 
1434 { 
1435 info_ptr->row_pointers = (png_bytepp)png_malloc(png_ptr, 
1436 info_ptr->height * png_sizeof(png_bytep)); 
1437 #ifdef PNG_FREE_ME_SUPPORTED 
1438 info_ptr->free_me |= PNG_FREE_ROWS; 
1439 #endif 
1440 for (row = 0; row < (int)info_ptr->height; row++) 
1441 { 
1442 info_ptr->row_pointers[row] = (png_bytep)png_malloc(png_ptr, 
1443 png_get_rowbytes(png_ptr, info_ptr)); 
1444 } 
1445 } 

Here you can see (info_ptr->free_me & PNG_FREE_ROWS) is set after the space for 
the pointers has been allocated, but _before_ the rows themselves have 
been allocated. If any of row allocations fail, a portion of the 
pointers in info_ptr->row_pointers are going to be unintialised junk, 


Rats. I liked the version number 1.2.34 as the final stable version number. 

Would this cure the defect? 

insert between 1439 and 1440 

for (row = 0; row < (int)info_ptr->height; row++) 
info_ptr->row_pointers[row] = NULL; 

Oh, never mind, your "memset" solution is more compact. 

Glenn 

(this is easy to control if an application will render two images an 
attacker will supply, the first one can poison the heap and succeed, the 
second triggers the fault, resulting in free(attacker_controlled). These 
can be exploitable (although its tricky) by setting up "fake" heap 
chunks, and pointing the system allocator at it. 

I've got a demo that works on my machine, I'm using setrlimit() to make 
allocation failure more reliable, but unless you're using the same 
distribution as me it may need tweaking, 3 required files attached. 

(note, i modified the pngs by hand so im cheating and using 
png_set_crc_action() instead of fixing them up, sorry!) 

$ gcc -ggdb3 -O0 pngdecode.c libpng-1.2.34/.libs/libpng.a -o pngdecode -lm -lz 
$ gdb -q ./pngdecode 
(gdb) r 
Breakpoint 1, png_default_error (png_ptr=0x80642d8, 
error_message=0x4b0d2bd2 "invalid literal/lengths set") at pngerror.c:218 
218 if (*error_message == '#') 
(gdb) c 
Breakpoint 1, png_default_error (png_ptr=0x80642d8, 
error_message=0x8061450 "Out of Memory!") at pngerror.c:218 
218 if (*error_message == '#') 
(gdb) bt 
#0 png_default_error (png_ptr=0x80642d8, error_message=0x8061450 "Out 
of Memory!") at pngerror.c:218 
#1 0x08056ff3 in png_error (png_ptr=0x80642d8, error_message=0x8061450 
"Out of Memory!") at pngerror.c:80 
#2 0x08056cea in png_malloc (png_ptr=0x80642d8, size=33132) at 
pngmem.c:442 
#3 0x0804c8c8 in png_read_png (png_ptr=0x80642d8, info_ptr=0x8064008, 
transforms=0, params=0x0) at pngread.c:1442 
(gdb) frame 3 
#3 0x0804c8c8 in png_read_png (png_ptr=0x80642d8, info_ptr=0x8064008, 
transforms=0, params=0x0) at pngread.c:1442 
1442 info_ptr->row_pointers[row] = 
(png_bytep)png_malloc(png_ptr, 
(gdb) p row 
$2 = 3993 
(gdb) p info_ptr->height 
$3 = 15173 

Memory allocation failed here when row < info_ptr->height, so the rest of the 
pointers in this array are unintialised junk. 

(gdb) c 
libpng error: Out of Memory! 
Program received signal SIGSEGV, Segmentation fault. 
0x48997fe9 in free () from /lib/tls/libc.so.6 
(gdb) bt 
#0 0x48997fe9 in free () from /lib/tls/libc.so.6 
#1 0x08056d97 in png_free_default (png_ptr=0x80642d8, ptr=0x5080) at pngmem.c:525 
#2 0x08056d76 in png_free (png_ptr=0x80642d8, ptr=0x5080) at pngmem.c:509 
#3 0x080494d9 in png_free_data (png_ptr=0x80642d8, info_ptr=0x8064008, mask=32767, num=-1) at png.c:571 
#4 0x080495a2 in png_info_destroy (png_ptr=0x80642d8, info_ptr=0x8064008) at png.c:598 
#5 0x0804c20b in png_read_destroy (png_ptr=0x80642d8, info_ptr=0x8064008, end_info_ptr=0x0) at pngread.c:1166 
#6 0x0804c12c in png_destroy_read_struct (png_ptr_ptr=0xffffce54, info_ptr_ptr=0xffffce50, end_info_ptr_ptr=0x0) 
at pngread.c:1107 
... 
(gdb) frame 3 
#3 0x080494d9 in png_free_data (png_ptr=0x80642d8, info_ptr=0x8064008, mask=32767, num=-1) at png.c:571 
571 png_free(png_ptr, info_ptr->row_pointers[row]); 
(gdb) p row 
$4 = 4028 
(gdb) p info_ptr->row_pointers[row] 
$5 = (png_byte *) 0x5080 <Address 0x5080 out of bounds> 

row is > 3993, where the allocation failure happened. 

I suppose the easiest fix is just to png_memset(foo, 0, n) row_pointers as soon as 
they're allocated, before free_me gets the ROW_POINTERS flag set. png_free 
handles free(NULL), so you dont have rely on the system allocator being sane. 
Oneliner below. 

$ diff -up pngread.c.orig pngread.c

--- pngread.c.orig 2009-02-04 03:33:27.605190000 +0000 
+++ pngread.c 2009-02-04 03:34:25.383958000 +0000 
@@ -1434,6 +1434,8 @@ png_read_png(png_structp png_ptr, png_in 
{ 
info_ptr->row_pointers = (png_bytepp)png_malloc(png_ptr, 
info_ptr->height * png_sizeof(png_bytep)); 
+ png_memset(info_ptr->row_pointers, 0, 
+ info_ptr->height * png_sizeof(png_bytep)); 
#ifdef PNG_FREE_ME_SUPPORTED 
info_ptr->free_me |= PNG_FREE_ROWS; 
#endif 

I'm not sure if you need additional logic to handle 16bit machines, like the 
unused logic in png_zalloc()? 

Dunno. I think my version (explicitly setting the pointers one by one) 
would be immune to pointer-size problems. 

Glenn 

Thanks, Tavis. 

-- 
------------------------------------- 
taviso at sdf.lonestar.org | finger me for my gpg key. 
------------------------------------------------------- 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/png-mng-security-archive/attachments/20090204/a787ad8d/attachment.html>
