Bug 623831 (CVE-2010-1519) - CVE-2010-1519 glpng: Two integer overflow vulnerabilities
Summary: CVE-2010-1519 glpng: Two integer overflow vulnerabilities
Keywords:
Status: CLOSED ERRATA
Alias: CVE-2010-1519
Product: Security Response
Classification: Other
Component: vulnerability
Version: unspecified
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Red Hat Product Security
QA Contact:
URL:
Whiteboard:
Depends On: 623832
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-08-12 21:59 UTC by Vincent Danen
Modified: 2021-10-19 09:13 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-10-19 09:13:27 UTC
Embargoed:


Attachments (Terms of Use)
PATCH: fix CVE-2010-1519 (4.48 KB, patch)
2010-09-10 12:45 UTC, Hans de Goede
no flags Details | Diff

Description Vincent Danen 2010-08-12 21:59:32 UTC
Secunia reported [1] two integer overflow issues in glpng 1.45 (and possibly earlier versions):

1) An integer overflow error within the "pngLoadRawF()" function in glpng.c can
be exploited to cause a heap-based buffer overflow by e.g. tricking a user into
opening a specially crafted PNG file in an application using the library.

2) An integer overflow error within the "pngLoadF()" function in glpng.c can be
exploited to cause a heap-based buffer overflow by e.g. tricking a user into
opening a specially crafted PNG file in an application using the library.

The upstream web site for glpng is currently not responding.  There is no word on an upstream- or community-provided patch as of yet.

[1] http://secunia.com/secunia_research/2010-87/

Comment 1 Vincent Danen 2010-08-12 22:00:18 UTC
Created libglpng tracking bugs for this issue

Affects: fedora-all [bug 623832]

Comment 2 Hans de Goede 2010-08-13 07:38:55 UTC
The advisory:
http://secunia.com/secunia_research/2010-87/

Is a bit scarce on details, it mentions posts to the vendor-sec list, did those include some more details perhaps ?

Comment 3 Vincent Danen 2010-08-13 15:03:36 UTC
Not much.  They did indicate that the integer overflow in pngLoadRoawF() was around line 304 and the integer overflow in pngLoadF() was around line 443.  They also indicated that there were other potential integer overflows in line 461 and 542, and also noted that the functions do not properly check if the return value is NULL, which could potentially be exploited to cause a crash.

That is the extent of any "additional" details provided.  Not overly helpful, I'm afraid.

Comment 16 Greg Roelofs 2010-09-17 01:21:28 UTC
Note that the attached patch appears to add some (small) memory leaks at 280, 282, 400 and 402.  Specifically, if png (and optionally info) succeeds but either info or endinfo fails, the memory allocated in the succeeding case(s) is leaked.

Comment 17 Hans de Goede 2010-09-17 04:05:23 UTC
(In reply to comment #16)
> Note that the attached patch appears to add some (small) memory leaks at 280,
> 282, 400 and 402.  Specifically, if png (and optionally info) succeeds but
> either info or endinfo fails, the memory allocated in the succeeding case(s) is
> leaked.

Thanks, but I knew already. The problem is that the png destroy function wants
all 3 pointers to do a destroy. So I see no clear way to free up the png and/or info when the endinfo allocation failed.

Note it may be that the destroy function will accept NULL pointers for info / endinfo. The documentation is unclear on this, and given the very small likely hood of this ever happening, I didn't bother to actual check the code. So I've deliberately chosen to leak some memory in this case rather then the destroy function not grokking a NULL ptr causing a crash (although once malloc's start failing the app is doomed anyway).

Regards,

Hans

Comment 18 Greg Roelofs 2010-09-17 05:03:17 UTC
> Note it may be that the destroy function will accept NULL pointers for info /
> endinfo. The documentation is unclear on this

In fact, libpng.txt includes an explicit example of precisely this:

    png_structp png_ptr = png_create_read_struct
       (PNG_LIBPNG_VER_STRING, (png_voidp)user_error_ptr,
        user_error_fn, user_warning_fn);
    if (!png_ptr)
        return (ERROR);
 
    png_infop info_ptr = png_create_info_struct(png_ptr);
    if (!info_ptr) 
    {
        png_destroy_read_struct(&png_ptr,
           (png_infopp)NULL, (png_infopp)NULL);
        return (ERROR); 
    }

    png_infop end_info = png_create_info_struct(png_ptr);
    if (!end_info) 
    { 
        png_destroy_read_struct(&png_ptr, &info_ptr,
          (png_infopp)NULL);
        return (ERROR);
    }

This example is present even in the 1.2.5 manual (2002), which is the oldest non-proprietary-format version still linked on libpng.org:  http://www.libpng.org/pub/png/libpng-1.2.5-manual.html .  I'm certain even 1.0 supported it; it's a fairly fundamental bit of logic.  I used it without ifdefs in the contrib/gregbook demo programs, and those date from 1999 (1.0.3 days).


Note You need to log in before you can comment on or make changes to this bug.