PNG and MNG/JNG image formats: home site

Email Archive: png-mng-implement (read-only)

Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

From: Glenn Randers-Pehrson <glennrp@co...> - 2007-10-05 18:32

At 11:03 AM 10/5/2007 -0700, bnason@playstation.sony.com wrote:

>pngset.c, png_set_iCCP(), lines 687-693:

>> png_strncpy(new_iccp_name, name, png_strlen(new_iccp_name)+1);
>
>That string copy is using the wrong size, it needs to be
>png_strlen(name) instead, right

Right.
Thanks, will fix in 1.2.22
Glenn

Thread View

Thread	Author	Date
[png-mng-implement] libpng 1.2.21 iCCP chunk handling bug	<bnason@pl...>

Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

From: Jeff Phillips <Jeff.Phillips@sa...> - 2007-10-05 18:31

I would concur with Ben and I didn't even catch that when I picked up
the new 1.2.21, nor 1.2.21rc3.

The allocation should include the +1, but the strcpy should not.

Jeff Phillips

-----Original Message-----
From: png-mng-implement-bounces@lists.sourceforge.net /> [mailto:png-mng-implement-bounces@lists.sourceforge.net] On Behalf Of
bnason@playstation.sony.com /> Sent: Friday, October 05, 2007 2:04 PM
To: png-mng-implement@lists.sourceforge.net /> Subject: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

Hi,

I think I found a memory corruption bug (and potential security problem)
in the 1.2.21 release of libpng. Sorry if this was already reported, but
I didn't see any mention of it on the archives.

It seems likely this is caused by an incorrect fix for the "Suspicious
`sizeof' line 694 of pngset.c" problem reported by Jeff Phillips about a
month ago.

http://sourceforge.net/mailarchive/forum.php?thread_name=3D5122753600C3E9=
4
F87FBDFFCC090D1FF0400EA68%40MERCMBX07.na.sas.com&forum_name=3Dpng-mng-imp=
l
ement
http://tinyurl.com/2h2vwd

pngset.c, png_set_iCCP(), lines 687-693:

> new_iccp_name=3D(png_charp)png_malloc_warn(png_ptr, =
png_strlen(name)+1);
> if (new_iccp_name =3D=3D NULL)
> {
> png_warning(png_ptr, "Insufficient memory to process iCCP chunk.");
> return;
> }
> png_strncpy(new_iccp_name, name, png_strlen(new_iccp_name)+1);

That string copy is using the wrong size, it needs to be
png_strlen(name) instead, right? That was the fix Jeff suggested
originally, it looks like it was just a copy-paste error or typo.

Thanks,
Ben

------------------------------------------------------------------------
-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
png-mng-implement mailing list
png-mng-implement@lists.sourceforge.net /> https://lists.sourceforge.net/lists/listinfo/png-mng-implement

Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

From: Glenn Randers-Pehrson <glennrp@co...> - 2007-10-05 18:32

At 11:03 AM 10/5/2007 -0700, bnason@playstation.sony.com wrote:

>pngset.c, png_set_iCCP(), lines 687-693:

>> png_strncpy(new_iccp_name, name, png_strlen(new_iccp_name)+1);
>
>That string copy is using the wrong size, it needs to be
>png_strlen(name) instead, right

Right.
Thanks, will fix in 1.2.22
Glenn

Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

From: Glenn Randers-Pehrson <glennrp@co...> - 2007-10-05 18:39

At 02:30 PM 10/5/2007 -0400, Jeff Phillips wrote:
>The allocation should include the +1, but the strcpy should not.

I think it's OK to copy the terminating NULL.

Glenn

Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

From: <bnason@pl...> - 2007-10-05 19:44

Hi,

I think it's not only OK, I think it would be a bug to not copy the null
terminating character. This is because png_malloc_warn() is not zero
filling the allocated string as far as I can tell, and strncpy() is not
going to add the null terminator for you in this case:

"If there is no null byte in the first n bytes of the array pointed to
by s2, the result will not be null-terminated."
- http://www.opengroup.org/pubs/online/7908799/xsh/strncpy.html

-Ben

Glenn Randers-Pehrson wrote:
> At 02:30 PM 10/5/2007 -0400, Jeff Phillips wrote:
>> The allocation should include the +1, but the strcpy should not.
>
> I think it's OK to copy the terminating NULL.
>
> Glenn

Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

From: Jeff Phillips <Jeff.Phillips@sa...> - 2007-10-05 19:47

You are correct, again. I was thinking that this memory was getting the
memset done on it. I'm trying to balance too many things at work today
and am not paying close attention to details :)

-----Original Message-----
From: png-mng-implement-bounces@lists.sourceforge.net /> [mailto:png-mng-implement-bounces@lists.sourceforge.net] On Behalf Of
bnason@playstation.sony.com /> Sent: Friday, October 05, 2007 3:44 PM
To: PNG/MNG implementation discussion list
Subject: Re: [png-mng-implement] libpng 1.2.21 iCCP chunk handling bug

Hi,

I think it's not only OK, I think it would be a bug to not copy the null
terminating character. This is because png_malloc_warn() is not zero
filling the allocated string as far as I can tell, and strncpy() is not
going to add the null terminator for you in this case:

"If there is no null byte in the first n bytes of the array pointed to
by s2, the result will not be null-terminated."
- http://www.opengroup.org/pubs/online/7908799/xsh/strncpy.html

-Ben

Glenn Randers-Pehrson wrote:
> At 02:30 PM 10/5/2007 -0400, Jeff Phillips wrote:
>> The allocation should include the +1, but the strcpy should not.
>=20
> I think it's OK to copy the terminating NULL.
>=20
> Glenn

------------------------------------------------------------------------
-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
png-mng-implement mailing list
png-mng-implement@lists.sourceforge.net /> https://lists.sourceforge.net/lists/listinfo/png-mng-implement

PNG and MNG/JNG image formats: home site

Email Archive: png-mng-implement (read-only)

Thread View

About SourceForge

Find Software

Develop Software

Community

Help