libPNG 1.2.5 stack-based buffer overflow and other code concerns
================================================================
Programs : libpng users including mozilla, konqueror, various e-mail clients, generally lots. Also reports that some versions of IE are vulnerable to some of the problems.
Severity : - A malicious website serving a malicious PNG file could compromise the browsers of visitors. - A malicious PNG could be sent via e-mail and compromise the e-mail viewer of the recipient. - For systems with user-providable images for "face browsers", a local system compromise could be possible via a malicious PNG.
CAN identifier(s): CAN-2004-0597 (the serious one), CAN-2004-0598, CAN-2004-0599
CERT VU#s : VU#388984 (the serious one), VU#236656, VU#160448, VU#477512, VU#817368, VU#286464
This advisory lists code flaws discovered by inspection of the libpng-1.2.5
code. Only the first one has been examined in practice to confirm
exploitability. The other flaws certainly warrant fixing.
A patch which should plug all these issues is appended beneath the advisory.
NOTE! This patch serves as demo purposes for the flaws only. An official
v1.2.6 libpng with an official, slightly different fix will be released by
the libpng team in parallel with this advisory.
1) Remotely exploitable stack-based buffer overrun in png_handle_tRNS
(pngrutil.c)
If a PNG file is of the correct format, a length check on PNG data is missed
prior to filling a buffer on the stack from the PNG data. The exact flaw would
seem to be a logic error; failure to bail out of a function after a warning
condition is hit, here:
if (!(png_ptr->mode & PNG_HAVE_PLTE))
{
/* Should be an error, but we can cope with it */ png_warning(png_ptr, "Missing PLTE before tRNS");
}
else if (length > (png_uint_32)png_ptr->num_palette)
{ png_warning(png_ptr, "Incorrect tRNS chunk length"); png_crc_finish(png_ptr, length);
return;
}
We can see, if the first warning condition is hit, the length check is missed
due to the use of an "else if".
It crashes both mozilla and konqueror.
A scarier possibility is targetted exploitation by e-mailing a nasty PNG to
someone who uses a graphical e-mail client to decode PNGs with a vulnerable
libpng.
2) Dangerous code in png_handle_sBIT (pngrutil.c) (Similar code in
png_handle_hIST).
Although seemingly not exploitable, there is dangerous code in this function.
It relies on checks scattered elsewhere in the code in order to not overflow
a 4-byte stack buffer. This line here should upper-bound the read onto the
stack to 4 bytes:
png_crc_read(png_ptr, buf, truelen);
3) Possible NULL-pointer crash in png_handle_iCCP (pngrutil.c) (this flaw is
duplicated in multiple other locations).
There are lots of lines such as these in the code:
Where "length" comes from the PNG. If length is set to UINT_MAX then length + 1 will equate to zero, leading to the PNG malloc routines to return NULL and
subsequent access to crash. These lengths are sometimes checked to ensure
they are smaller that INT_MAX, but it is not clear that all code paths perform
this check, i.e. png_push_read_chunk in pngpread.c does not do this check
(this is progressive reading mode as used by browsers).
4) Theoretical integer overflow in allocation in png_handle_sPLT (pngrutil.c)
This isn't likely to cause problems in practice, but there's the possibility
of an integer overflow during this allocation:
A PNG with excessive height may cause an integer overflow on a memory
allocation and subsequent crash allocating row pointers. This line is possibly
faulty; I can't see anywhere that enforces a maximum PNG height:
There are many lines like the following, which are prone to integer overflow:
if (png_ptr->push_length + 4 > png_ptr->buffer_size)
It is not clear how dangerous this is.
7) Other flaws.
There is broad potential for other integer overflows which I have not spotted -
the amount of integer arithmetic surrounding buffer handling is large,
unfortunately.
CESA-2004-001 - rev 3
Chris Evans
<email address hidden>
[Advertisement: I am interested in moving into a security related field
full-time. E-mail me to discuss.]
diff -ru libpng-1.2.5/png.h libpng-1.2.5.fix/png.h
--- libpng-1.2.5/png.h 2002-10-03 12:32:26.000000000 +0100
+++ libpng-1.2.5.fix/png.h 2004-07-13 23:18:10.000000000 +0100
@@ -835,6 +835,9 @@
/* Maximum positive integer used in PNG is (2^31)-1 */
#define PNG_MAX_UINT ((png_uint_32)0x7fffffffL)
+/* Constraints on width, height, (2 ^ 24) - 1*/
+#define PNG_MAX_DIMENSION 16777215
+
/* These describe the color_type field in png_info. */
/* color type masks */
#define PNG_COLOR_MASK_PALETTE 1
diff -ru libpng-1.2.5/pngpread.c libpng-1.2.5.fix/pngpread.c
--- libpng-1.2.5/pngpread.c 2002-10-03 12:32:28.000000000 +0100
+++ libpng-1.2.5.fix/pngpread.c 2004-07-13 23:03:58.000000000 +0100
@@ -209,6 +209,8 @@
Versions of packages libpng12-0 depends on:
ii libc6 2.3.2.ds1-14 GNU C Library: Shared libraries an
ii zlib1g 1:1.2.1.1-5 compression library - runtime
-- no debconf information
--
Obsig: developing a new sig
Message-ID: <email address hidden>
Date: Wed, 4 Aug 2004 21:46:21 +0200
From: "J.H.M. Dassen (Ray)" <email address hidden>
To: Debian Bug Tracking System <email address hidden>
Subject: [CAN-2004-0597, CAN-2004-0598,
CAN-2004-0599] stack-based buffer overflow and other code concerns
Package: libpng, libpng3
Version: 1.2.5.0-6
Severity: grave
Tags: security upstream woody sarge sid patch
Justification: Remotely exploitable stack-based buffer overrun
http:// scary.beasts. org/security/ CESA-2004- 001.txt : ------- ------- ------- ------- ------- ------- ------- ------- ------- ------- -
-------
CESA-2004-001 - rev 3
libPNG 1.2.5 stack-based buffer overflow and other code concerns ======= ======= ======= ======= ======= ======= ======= ======= =
=======
Programs : libpng users including mozilla, konqueror, various e-mail
clients, generally lots. Also reports that some versions of
IE are vulnerable to some of the problems.
compromise the browsers of visitors.
- A malicious PNG could be sent via e-mail and compromise
the e-mail viewer of the recipient.
- For systems with user-providable images for "face
browsers" , a local system compromise could be possible via
a malicious PNG.
CAN- 2004-0599
VU# 477512, VU#817368, VU#286464
Severity : - A malicious website serving a malicious PNG file could
CAN identifier(s): CAN-2004-0597 (the serious one), CAN-2004-0598,
CERT VU#s : VU#388984 (the serious one), VU#236656, VU#160448,
This advisory lists code flaws discovered by inspection of the libpng-1.2.5
code. Only the first one has been examined in practice to confirm
exploitability. The other flaws certainly warrant fixing.
A patch which should plug all these issues is appended beneath the advisory.
NOTE! This patch serves as demo purposes for the flaws only. An official
v1.2.6 libpng with an official, slightly different fix will be released by
the libpng team in parallel with this advisory.
1) Remotely exploitable stack-based buffer overrun in png_handle_tRNS
(pngrutil.c)
If a PNG file is of the correct format, a length check on PNG data is missed
prior to filling a buffer on the stack from the PNG data. The exact flaw would
seem to be a logic error; failure to bail out of a function after a warning
condition is hit, here:
if (!(png_ptr->mode & PNG_HAVE_PLTE))
png_warning( png_ptr, "Missing PLTE before tRNS"); 32)png_ ptr->num_ palette)
png_warning( png_ptr, "Incorrect tRNS chunk length");
png_crc_ finish( png_ptr, length);
{
/* Should be an error, but we can cope with it */
}
else if (length > (png_uint_
{
return;
}
We can see, if the first warning condition is hit, the length check is missed
due to the use of an "else if".
A PNG crafted to trip this is available at scary.beasts. org/misc/ pngtest_ bad.png
http://
It crashes both mozilla and konqueror.
A scarier possibility is targetted exploitation by e-mailing a nasty PNG to
someone who uses a graphical e-mail client to decode PNGs with a vulnerable
libpng.
2) Dangerous code in png_handle_sBIT (pngrutil.c) (Similar code in
png_handle_hIST).
Although seemingly not exploitable, there is dangerous code in this function.
It relies on checks scattered elsewhere in the code in order to not overflow
a 4-byte stack buffer. This line here should upper-bound the read onto the
stack to 4 bytes:
png_ crc_read( png_ptr, buf, truelen);
3) Possible NULL-pointer crash in png_handle_iCCP (pngrutil.c) (this flaw is
duplicated in multiple other locations).
There are lots of lines such as these in the code:
chunkdata = (png_charp) png_malloc( png_ptr, length + 1);
Where "length" comes from the PNG. If length is set to UINT_MAX then length + 1 will equate to zero, leading to the PNG malloc routines to return NULL and
subsequent access to crash. These lengths are sometimes checked to ensure
they are smaller that INT_MAX, but it is not clear that all code paths perform
this check, i.e. png_push_read_chunk in pngpread.c does not do this check
(this is progressive reading mode as used by browsers).
4) Theoretical integer overflow in allocation in png_handle_sPLT (pngrutil.c)
This isn't likely to cause problems in practice, but there's the possibility
of an integer overflow during this allocation:
new_ palette. entries = (png_sPLT_ entryp) png_malloc( nentries * sizeof( png_sPLT_ entry)) ;
png_ptr, new_palette.
5) Integer overflow in png_read_png (pngread.c)
A PNG with excessive height may cause an integer overflow on a memory
allocation and subsequent crash allocating row pointers. This line is possibly
faulty; I can't see anywhere that enforces a maximum PNG height:
info_ ptr->row_ pointers = (png_bytepp) png_malloc( png_ptr,
info_ ptr->height * sizeof(png_bytep));
6) Integer overflows during progressive reading.
There are many lines like the following, which are prone to integer overflow:
if (png_ptr- >push_length + 4 > png_ptr- >buffer_ size)
It is not clear how dangerous this is.
7) Other flaws.
There is broad potential for other integer overflows which I have not spotted -
the amount of integer arithmetic surrounding buffer handling is large,
unfortunately.
CESA-2004-001 - rev 3
Chris Evans
<email address hidden>
[Advertisement: I am interested in moving into a security related field
full-time. E-mail me to discuss.]
diff -ru libpng-1.2.5/png.h libpng- 1.2.5.fix/ png.h 1.2.5.fix/ png.h 2004-07-13 23:18:10.000000000 +0100 32)0x7fffffffL)
--- libpng-1.2.5/png.h 2002-10-03 12:32:26.000000000 +0100
+++ libpng-
@@ -835,6 +835,9 @@
/* Maximum positive integer used in PNG is (2^31)-1 */
#define PNG_MAX_UINT ((png_uint_
+/* Constraints on width, height, (2 ^ 24) - 1*/ MASK_PALETTE 1 1.2.5/pngpread. c libpng- 1.2.5.fix/ pngpread. c 1.2.5/pngpread. c 2002-10-03 12:32:28.000000000 +0100 1.2.5.fix/ pngpread. c 2004-07-13 23:03:58.000000000 +0100
+#define PNG_MAX_DIMENSION 16777215
+
/* These describe the color_type field in png_info. */
/* color type masks */
#define PNG_COLOR_
diff -ru libpng-
--- libpng-
+++ libpng-
@@ -209,6 +209,8 @@
+ if (png_ptr-
+ png_error(png_ptr, "Invalid chunk length.");
@@ -638,6 +640,8 @@
+ if (png_ptr-
+ png_error(png_ptr, "Invalid chunk length.");
diff -ru libpng-
--- libpng-
+++ libpng-
@@ -350,7 +350,11 @@
png_
width = png_get_ uint_32( buf); _type = buf[10]; t)png_ptr- >channels;
+ if (width > PNG_MAX_DIMENSION)
+ png_error(png_ptr, "Width is too large");
height = png_get_uint_32(buf + 4);
+ if (height > PNG_MAX_DIMENSION)
+ png_error(png_ptr, "Height is too large");
bit_depth = buf[8];
color_type = buf[9];
compression
@@ -675,7 +679,7 @@
else
truelen = (png_size_
- if (length != truelen)
png_warning( png_ptr, "Incorrect sBIT chunk length");
png_crc_ finish( png_ptr, length);
png_ warning( png_ptr, "Missing PLTE before tRNS"); 32)png_ ptr->num_ palette) 32)png_ ptr->num_ palette || PALETTE_ LENGTH)
png_ warning( png_ptr, "Incorrect tRNS chunk length");
png_ crc_finish( png_ptr, length); hIST(png_ structp png_ptr, png_infop info_ptr, png_uint_32 length) PNG_MAX_ PALETTE_ LENGTH] ;
+ if (length != truelen || length > 4)
{
@@ -1244,7 +1248,8 @@
/* Should be an error, but we can cope with it */
}
- else if (length > (png_uint_
+ if (length > (png_uint_
+ length > PNG_MAX_
{
@@ -1400,7 +1405,7 @@
void /* PRIVATE */
png_handle_
{
- int num, i;
+ unsigned int num, i;
png_uint_16 readbuf[
png_debug(1, "in png_handle_ hIST\n" );
@@ -1426,8 +1431,8 @@
return;
}
- num = (int)length / 2 ; >num_palette) >num_palette || num > PNG_MAX_ PALETTE_ LENGTH)
png_warning( png_ptr, "Incorrect hIST chunk length");
png_crc_ finish( png_ptr, length);
png_read_ data(png_ ptr, chunk_length, 4);
png_ptr- >idat_size = png_get_ uint_32( chunk_length) ;
- if (num != png_ptr-
+ num = length / 2 ;
+ if (num != png_ptr-
{
@@ -2868,6 +2873,9 @@
+ if (png_ptr->idat_size > PNG_MAX_UINT)
png_reset_ crc(png_ ptr);
png_crc_ read(png_ ptr, png_ptr- >chunk_ name, 4); png_ptr- >chunk_ name, (png_bytep) png_IDAT, 4)) ------- ------- ------- ------- ------- ------- ------- ------- ------- ------- -
+ png_error(png_ptr, "Invalid chunk length.");
+
if (png_memcmp(
-------
libpng 1.2.6rc1 (available from http:// sourceforge. net/projects/ libpng)
addresses part of these issues.
-- System Information: en_US.ISO8859- 1
Debian Release: 3.1
APT prefers unstable
APT policy: (800, 'unstable'), (750, 'experimental'), (500, 'testing')
Architecture: i386 (i686)
Kernel: Linux 2.4.27-rc5
Locale: LANG=C, LC_CTYPE=
Versions of packages libpng12-0 depends on:
ii libc6 2.3.2.ds1-14 GNU C Library: Shared libraries an
ii zlib1g 1:1.2.1.1-5 compression library - runtime
-- no debconf information
--
Obsig: developing a new sig