libpng extra row (CVE-2010-1205)
Moved from d0cs4vage.blogspot.com to here
This bug is a fairly straightforward one. I am interested in it for two main reasons. The first, and smaller reason, is that it’s an interesting bug in that two parties are at fault if an application is vulnerable: the makers of the application and libpng itself. Each had to do something wrong in order for this to be an actual problem. The second, larger reason why I am so interested in this bug is because I had fuzzed into this bug back in April and sat on it too long. What happened next? Lo and behold, I wasn’t the only one looking at pngs – someone else reported it (first public reference: Google Chrome Issue, see also the alert on the libpng home page, and most recently, a Mozilla Security Advisory). Props to the guy who reported it (Aki Helin). With that in mind, here’s a brief description of the vuln: an extra row in the image data is bad sarcasm.
Actually, it’s slightly more complicated than that. Libpng uses callbacks to
aid with progressive reading of a png. That way, as the image rows become
available, the application can handle them. The root problem lies in the
fact that libpng versions prior to the fix would call the row_callback()
function in the application for every row without checking to see if
more rows had been processed than the declared height in the IHDR chunk
(which is what memory allocations are [usually] based on). Since libpng
didn’t check the number of rows processed (or the number of times the
row_callback() was called), the last place to
catch this bug would be in the application. If the application checked
how many times its callback had been called against the height of the png
stored in the png_ptr, it could bail when
too many rows were received. But, libpng has been around for quite a while
(15+ years), so everyone trusts it and never does their own checks, merely
copying the inflated image rows into their own buffers, using a calculation
based on their base buffer address, image width, row number, and the number
of bytes per pixel.
Below is a snippet of Firefox’s row_callback function. The statement that
copies the image row data to the application’s buffer is on line 822:
// Firefox 3.6.3
// modules/libpr0n/decoders/png/nsPNGDecoder.cpp <-- who named this module??
725 void
726 row_callback(png_structp png_ptr, png_bytep new_row,
727 png_uint_32 row_num, int pass)
728 {
...
762 if (new_row) {
...
791 switch (decoder->format) {
...
819 case gfxASurface::ImageFormatARGB32:
820 {
821 for (PRUint32 x=width; x>0; --x) {
822 *cptr32++ = GFX_PACKED_PIXEL(line[3], line[0], line[1], line[2]);
823 if (line[3] != 0xff)
824 rowHasNoAlpha = PR_FALSE;
825 line += 4;
826 }
827 }
828 break;
...
833 }
...
851 }
852 }
Below is a snippet of Google Chrome’s row_callback. The destination address
in the code below is calculated on line 356, and the statement that copies
the image row data to the destination is on line 360:
// Google Chrome, r44471
// gfx/codec/png_codec.cc
338 void DecodeRowCallback(png_struct* png_ptr, png_byte* new_row,
339 png_uint_32 row_num, int pass) {
340 PngDecoderState* state = static_cast<PngDecoderState*>(
341 png_get_progressive_ptr(png_ptr));
342
343 DCHECK(pass == 0) << "We didn't turn on interlace handling, but libpng is "
344 "giving us interlaced data.";
345 if (static_cast<int>(row_num) > state->height) {
346 NOTREACHED() << "Invalid row";
347 return;
348 }
349
350 unsigned char* base = NULL;
351 if (state->bitmap)
352 base = reinterpret_cast<unsigned char*>(state->bitmap->getAddr32(0, 0));
353 else if (state->output)
354 base = &state->output->front();
355
356 unsigned char* dest = &base[state->width * state->output_channels * row_num];
357 if (state->row_converter)
358 state->row_converter(new_row, state->width, dest, &state->is_opaque);
359 else
360 memcpy(dest, new_row, state->width * state->output_channels);
361 }
Well, enough of the rant-ish speaking. Below is how the libpng code worked before the fix:
First, the function png_process_data() is called by the application:
// pngpread.c
30 void PNGAPI
31 png_process_data(png_structp png_ptr, png_infop info_ptr,
32 png_bytep buffer, png_size_t buffer_size)
33 {
34 if (png_ptr == NULL || info_ptr == NULL)
35 return;
36
37 png_push_restore_buffer(png_ptr, buffer, buffer_size);
38
39 while (png_ptr->buffer_size)
40 {
41 png_process_some_data(png_ptr, info_ptr);
42 }
43 }
This ends up calling png_process_some_data(), which iterates through and
processes each chunk. Once the IDAT chunk (the chunk where the image data
[and extra row] are stored) has been read and handled (via line 62), it is
then in the PNG_READ_IDAT_MODE. At this point, it then drops down into the
case at line 68, calling png_push_read_IDAT():
// pngpread.c
45 /* What we do with the incoming data depends on what we were previously
46 * doing before we ran out of data...
47 */
48 void /* PRIVATE */
49 png_process_some_data(png_structp png_ptr, png_infop info_ptr)
50 {
51 if (png_ptr == NULL)
52 return;
53
54 switch (png_ptr->process_mode)
55 {
...
62 case PNG_READ_CHUNK_MODE:
63 {
64 png_push_read_chunk(png_ptr, info_ptr);
65 break;
66 }
67
68 case PNG_READ_IDAT_MODE:
69 {
70 png_push_read_IDAT(png_ptr);
71 break;
72 }
...
109 }
110 }
The png_push_read_IDAT() function is a bit longer, so I’m only going to
show one relevant part:
// pngpread.c
735 void /* PRIVATE */
736 png_push_read_IDAT(png_structp png_ptr)
737 {
738 PNG_IDAT;
...
765 if (png_ptr->idat_size && png_ptr->save_buffer_size)
766 {
767 png_size_t save_size;
768
769 if (png_ptr->idat_size < (png_uint_32)png_ptr->save_buffer_size)
770 {
771 save_size = (png_size_t)png_ptr->idat_size;
772
773 /* Check for overflow */
774 if ((png_uint_32)save_size != png_ptr->idat_size)
775 png_error(png_ptr, "save_size overflowed in pngpread");
776 }
777 else
778 save_size = png_ptr->save_buffer_size;
779
780 png_calculate_crc(png_ptr, png_ptr->save_buffer_ptr, save_size);
781
782 if (!(png_ptr->flags & PNG_FLAG_ZLIB_FINISHED))
783 png_process_IDAT_data(png_ptr, png_ptr->save_buffer_ptr, save_size);
784
785 png_ptr->idat_size -= save_size;
786 png_ptr->buffer_size -= save_size;
787 png_ptr->save_buffer_size -= save_size;
788 png_ptr->save_buffer_ptr += save_size;
789 }
...
826 }
Basically, this function gets called multiple times, going
down a different code path depending on what state the
png_ptr->mode is in. One of the code paths makes
a call to png_process_IDAT_data(), which
inflates the compressed image data and begins the row processing. Notice
how libpng never checks to see if the number of rows processed is greater
than the declared height in the IHDR chunk:
// pngpread.c
828 void /* PRIVATE */
829 png_process_IDAT_data(png_structp png_ptr, png_bytep buffer,
830 png_size_t buffer_length)
831 {
832 int ret;
833
834 if ((png_ptr->flags & PNG_FLAG_ZLIB_FINISHED) && buffer_length)
835 png_benign_error(png_ptr, "Extra compression data");
836
837 png_ptr->zstream.next_in = buffer;
838 png_ptr->zstream.avail_in = (uInt)buffer_length;
839 for (;;)
840 {
841 ret = inflate(&png_ptr->zstream, Z_PARTIAL_FLUSH);
842 if (ret != Z_OK)
843 {
844 if (ret == Z_STREAM_END)
845 {
846 if (png_ptr->zstream.avail_in)
847 png_benign_error(png_ptr, "Extra compressed data");
848
849 if (!(png_ptr->zstream.avail_out))
850 {
851 png_push_process_row(png_ptr);
852 }
853
854 png_ptr->mode |= PNG_AFTER_IDAT;
855 png_ptr->flags |= PNG_FLAG_ZLIB_FINISHED;
856 break;
857 }
858 else if (ret == Z_BUF_ERROR)
859 break;
860
861 else
862 png_error(png_ptr, "Decompression Error");
863 }
864 if (!(png_ptr->zstream.avail_out))
865 {
866 if ((
867 #ifdef PNG_READ_INTERLACING_SUPPORTED
868 png_ptr->interlaced && png_ptr->pass > 6) ||
869 (!png_ptr->interlaced &&
870 #endif
871 png_ptr->row_number == png_ptr->num_rows))
872 {
873 if (png_ptr->zstream.avail_in)
874 png_warning(png_ptr, "Too much data in IDAT chunks");
875 png_ptr->flags |= PNG_FLAG_ZLIB_FINISHED;
876 break;
877 }
878 png_push_process_row(png_ptr);
879 png_ptr->zstream.avail_out =
880 (uInt) PNG_ROWBYTES(png_ptr->pixel_depth,
881 png_ptr->iwidth) + 1;
882 png_ptr->zstream.next_out = png_ptr->row_buf;
883 }
884
885 else
886 break;
887 }
888 }
As soon as a complete row is available, it calls the png_push_process_row()
function (I’m leaving out the interlaced code for simplicity):
// pngpread.c
890 void /* PRIVATE */
891 png_push_process_row(png_structp png_ptr)
892 {
893 png_ptr->row_info.color_type = png_ptr->color_type;
894 png_ptr->row_info.width = png_ptr->iwidth;
895 png_ptr->row_info.channels = png_ptr->channels;
896 png_ptr->row_info.bit_depth = png_ptr->bit_depth;
897 png_ptr->row_info.pixel_depth = png_ptr->pixel_depth;
898
899 png_ptr->row_info.rowbytes = PNG_ROWBYTES(png_ptr->row_info.pixel_depth,
900 png_ptr->row_info.width);
901
902 png_read_filter_row(png_ptr, &(png_ptr->row_info),
903 png_ptr->row_buf + 1, png_ptr->prev_row + 1,
904 (int)(png_ptr->row_buf[0]));
905
906 png_memcpy(png_ptr->prev_row, png_ptr->row_buf, png_ptr->rowbytes + 1);
907
908 if (png_ptr->transformations || (png_ptr->flags&PNG_FLAG_STRIP_ALPHA))
909 png_do_read_transformations(png_ptr);
910
911 #ifdef PNG_READ_INTERLACING_SUPPORTED
...
1088 #endif
1089 {
1090 png_push_have_row(png_ptr, png_ptr->row_buf + 1);
1091 png_read_push_finish_row(png_ptr);
1092 }
1093 }
Notice the two calls on line 1090 and 1091. The first call,
png_push_have_row(), calls the row_callback function defined in the
application:
// pngpread.c
1682 void /* PRIVATE */
1683 png_push_have_row(png_structp png_ptr, png_bytep row)
1684 {
1685 if (png_ptr->row_fn != NULL)
1686 (*(png_ptr->row_fn))(png_ptr, row, png_ptr->row_number,
1687 (int)png_ptr->pass);
1688 }
The second call, png_read_push_finish_row(),
merely increments png_ptr->row_number (it does more post-processing for
interlaced images as well):
// pngpread.c
1095 void /* PRIVATE */
1096 png_read_push_finish_row(png_structp png_ptr)
1097 {
...
1117 png_ptr->row_number++;
...
1157 }
So what is the error? The number of actual rows in the image
data is never checked against the rows declared in the IHDR
chunk. When libpng fixed this bug, they rewrote most of the
png_process_IDAT_data() function. The critical fix they added was this:
// pngpread.c, libpng-1.4.3
827 void /* PRIVATE */
828 png_process_IDAT_data(png_structp png_ptr, png_bytep buffer,
829 png_size_t buffer_length)
830 {
...
845 while (png_ptr->zstream.avail_in > 0 &&
846 !(png_ptr->flags & PNG_FLAG_ZLIB_FINISHED))
847 {
...
891 /* Did inflate output any data? */
892 if (png_ptr->zstream.next_out != png_ptr->row_buf)
893 {
894 /* Is this unexpected data after the last row?
895 * If it is, artificially terminate the LZ output
896 * here.
897 */
898 if (png_ptr->row_number >= png_ptr->num_rows ||
899 png_ptr->pass > 6)
900 {
901 /* Extra data. */
902 png_warning(png_ptr, "Extra compressed data in IDAT");
903 png_ptr->flags |= PNG_FLAG_ZLIB_FINISHED;
904 /* Do no more processing; skip the unprocessed
905 * input check below.
906 */
907 return;
908 }
...
918 }
...
926 }
Notice how it has the explicit check against the row number and the height
of the image on line 898. Libpng now only calls the row_callback function
at a maximum height times.
I think this is a prime example about trusting third-party code. Like a firearm safety instructor told me once about using the safety, “we use them, but we don’t trust them”. I think that is how third-party code should be used.