Menu

#56 optipng <= 0.7.5 heap buffer overflow pngxrbmp.c bmp_read_rows

v1.0 (example)
closed-fixed
security (6)
9
2016-04-05
2016-01-03
Henri Salo
No

Optipng is affected by a read/write out of bounds when processing malformed image files. This issue could be abused on server side applications that use optipng as external program for image processing. This issue was found with using american fuzzy lop.

Malicious input file: http://bugs.fi/media/afl/optipng/1/optipng-001.png
SHA1: 7387fbf79554068af982b038614ba94ed1cd55e8
Gdb backtrace: http://bugs.fi/media/afl/optipng/1/backtrace.txt
ASAN processing log: http://bugs.fi/media/afl/optipng/1/asan.txt

Optipng is used as external program in e.g. these software:

https://github.com/heyday/silverstripe-optimisedimage
https://github.com/jkphl/iconizr.git
https://github.com/christophlehmann/assetprocessor
https://github.com/psliwa/image-optimizer

Timeline:

2015-12-12: Reported to vendor via email (no reply yet)
2016-01-03: Emailed again to vendor and asked how to proceed
2016-01-03: Requested CVE identifier
2016-01-03: Created private item to issue tracker

Planning to announce this publicly this month (2016-01) if agreed with vendor.

Discussion

  • Ramona Truta

    Ramona Truta - 2016-01-20
    • assigned_to: Ramona Truta
     
  • Ramona Truta

    Ramona Truta - 2016-01-20

    Confirmed.

     
  • Ramona Truta

    Ramona Truta - 2016-01-25

    Thank you very much, Henri, for your report. Here is the fix:

    diff --git a/src/pngxtern/pngxrbmp.c b/src/pngxtern/pngxrbmp.c
    index 00000000..ffffffff 100644
    --- a/src/pngxtern/pngxrbmp.c
    +++ b/src/pngxtern/pngxrbmp.c
    @@ -320,7 +320,8 @@ bmp_read_rows(png_bytepp begin_row, png_bytepp end_row, size_t row_size,
                       crtn = 0;
                       ++result;
                    }
    -               bmp_memset_fn(*crt_row, crtn, 0, dcrtn - crtn);
    +               if (crt_row != end_row)
    +                  bmp_memset_fn(*crt_row, crtn, 0, dcrtn - crtn);
                 }
                 else  /* b2 >= 3 bytes in absolute mode */
                 {
    

    This is planned to go straight into the upcoming OptiPNG version 0.7.6.

     
  • Henri Salo

    Henri Salo - 2016-02-18

    When do you plan to release 0.7.6 version?

     
  • Ramona Truta

    Ramona Truta - 2016-02-19

    I just posted one more fix at https://sourceforge.net/p/optipng/bugs/57/

    Both of the images that you reported are now handled correctly with these two fixes, although I am still not entirely sure that I have covered all the cases.

     
  • Henri Salo

    Henri Salo - 2016-02-22

    I have applied patches against current hg version and continued fuzzing. No crashes found yet after 24h of fuzzing. I will report back if I find something or by 2016-03-05.

     
  • Ramona Truta

    Ramona Truta - 2016-02-24

    Here is an updated fix. It addresses both this issue and the one reported in https://sourceforge.net/p/optipng/bugs/59/

    diff --git a/src/pngxtern/pngxrbmp.c b/src/pngxtern/pngxrbmp.c
    index 00000000..ffffffff 100644
    --- a/src/pngxtern/pngxrbmp.c
    +++ b/src/pngxtern/pngxrbmp.c
    @@ -269,8 +269,7 @@ bmp_read_rows(png_bytepp begin_row, png_bytepp end_row, size_t row_size,
              bmp_memset_fn = bmp_rle4_memset;
              bmp_fread_fn = bmp_rle4_fread;
           }
    -      crt_row = begin_row;
    -      for ( ; ; )
    +      for (crt_row = begin_row; crt_row != end_row; )
           {
              ch = getc(stream); b1 = (unsigned int)ch;
              ch = getc(stream); b2 = (unsigned int)ch;
    @@ -311,16 +310,17 @@ bmp_read_rows(png_bytepp begin_row, png_bytepp end_row, size_t row_size,
                    if (ch == EOF)
                       break;
                    dcrtn = (b1 < endn - crtn) ? (crtn + b1) : endn;
    -               if (b2 > (size_t)((end_row - crt_row) * inc))
    -                  b2 = (unsigned int)((end_row - crt_row) * inc);
                    for ( ; b2 > 0; --b2)
                    {
                       bmp_memset_fn(*crt_row, crtn, 0, endn - crtn);
                       crt_row += inc;
                       crtn = 0;
                       ++result;
    +                  if (crt_row == end_row)
    +                      break;
                    }
    -               bmp_memset_fn(*crt_row, crtn, 0, dcrtn - crtn);
    +               if (crt_row != end_row)
    +                  bmp_memset_fn(*crt_row, crtn, 0, dcrtn - crtn);
                 }
                 else  /* b2 >= 3 bytes in absolute mode */
                 {
    
     
  • Henri Salo

    Henri Salo - 2016-03-13

    I do not have access to issue 59. After applying patches from 56 and 57 I still notice crash with e.g. http://bugs.fi/media/afl/optipng/1/optipng-002.png

    What is the status of this case?

     
  • Ramona Truta

    Ramona Truta - 2016-03-21

    The author of issue 59 has opened the CVE-2016-2191. It's a different test case, but addressing the same problem, and resolved by the same fix.

    I retried the test case you posted at http://bugs.fi/media/afl/optipng/2/optipng-002.png and it works for me, both on Mac and Linux.

    If all goes well, I should be able to make a new release sometime this week.

     
  • Ramona Truta

    Ramona Truta - 2016-03-21

    For the record, I have one more fix:

    diff --git a/src/pngxtern/pngxrbmp.c b/src/pngxtern/pngxrbmp.c
    index 562b3dd23b..ca45f61429 100644
    --- a/src/pngxtern/pngxrbmp.c
    +++ b/src/pngxtern/pngxrbmp.c
    @@ -300,6 +300,7 @@ bmp_read_rows(png_bytepp begin_row, png_bytepp end_row, size_t row_size,
                 {
                    bmp_memset_fn(*crt_row, crtn, 0, endn - crtn);
                    crt_row += inc;
    +               crtn = 0;
                    result = (begin_row <= end_row) ?
                       (end_row - begin_row) : (begin_row - end_row);
                    break;  /* the rest is wiped out at the end */
    
     
  • Ramona Truta

    Ramona Truta - 2016-04-04
    • status: open --> closed-fixed
     
  • Ramona Truta

    Ramona Truta - 2016-04-04

    Fixed in version 0.7.6.

     
  • Henri Salo

    Henri Salo - 2016-04-04

    Thank you for your efforts and have a nice week :) If you want feel free to make issues #56 and #57 public.

     
  • Ramona Truta

    Ramona Truta - 2016-04-05
    • private: Yes --> No
     
  • Ramona Truta

    Ramona Truta - 2016-04-05

    I removed the "private" flag. Thanks again for the report and feedback.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.