New issue
Advanced search Search tips

Issue 761608 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 771075
Owner:
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: ----
Type: Bug



Sign in to add a comment

Security: Security Vulnerability in libpng

Reported by kushal89...@gmail.com, Sep 2 2017

Issue description

VULNERABILITY DETAILS

The libpng copy that's bundled in chromium is currently at version 1.6.22. This version is vulnerable to a memory leak as described on the libpng home page.
See the topmost Vulnerability warning on the the LibPNG website, http://www.libpng.org/pub/png/libpng.html

Solution: libpng should be updated to 1.6.32.


 
LibPNG Website Details.tiff
191 KB Download
Chrome LibPng Current State.tiff
316 KB Download
Components: Internals>Images>Codecs
The issue described atop the library's page notes "1.6.31 added png_handle_eXIf(), which has a null-pointer-dereference bug as well as a potential memory leak".
Cc: scroggo@chromium.org
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Owner: msarett@chromium.org
Status: Assigned (was: Unconfirmed)
+msarett, +scroggo: are we vulnerable to the memory leak here? Can you coordinate updating libpng if necessary?
(To be clear, the bug fixed in 1.6.32 was introduced by 1.6.31, so I don't think there's anything to do here.)
Hello @elawre..., @dominickn, Google Product Security Team,

Good Morning.

I would like to point out some other vulnerabilities fixed by libPNG "before" v.1.6.31 which might seem comparatively more critical to fix in Chromium.

Also from https://cs.chromium.org/chromium/src/third_party/libpng/README.chromium?sq=package:chromium&dr&l=20, I noticed that Chromium seems to follow and pick up code changes from github.com/glennrp/libpng so I would like to share some findings for the same. They are as follows: -

1) https://github.com/glennrp/libpng/issues/153
2) https://github.com/glennrp/libpng/commit/4f31b7f242ecbc765d67920b11560e24445f8496
3) https://github.com/glennrp/libpng/issues/139
4) https://github.com/glennrp/libpng/issues/133

All of the aforementioned issues have been diligently fixed in LibPNG source.

Several more can be found in libPNG's changelog, affecting functions/libraries "supported" in current Chromium build. I can share them too if needed.

Eagerly awaiting your response.

Thanking You,

Yours Sincerely,
Kushal Arvind Shah.
Security Researcher | Fortinet's FortiGuard Labs.
Cc: -scroggo@chromium.org
Owner: scroggo@chromium.org
(FYI, msarett no longer works on Chromium.)

As pointed out in #3, we're not affected by the memory leak brought up in the original bug/#1.

The only vulnerability on libpng's website that applies to the version Chromium has is in png_set_text_2 (affects up to 1.6.26), but this method is not used by Chromium.

> 1) https://github.com/glennrp/libpng/issues/153
> 2) https://github.com/glennrp/libpng/commit/4f31b7f242ecbc765d67920b11560e24445f8496

Just FYI: (2) is the fix for (1).

> 3) https://github.com/glennrp/libpng/issues/139

Hmm, this one was closed but without a specified fix. Many of the errors are in test apps, but some are in libpng itself.

> 4) https://github.com/glennrp/libpng/issues/133

I see a fix for this one here: https://github.com/glennrp/libpng/pull/135/commits/7dc03292502cf91a116766d2ab2c4f8fc23e4ab4. This is a memory leak, which shouldn't be a security issue (if a website forces us to allocate too much memory we'll safely crash the renderer).

Again, the libpng maintainers don't seem to think these are security issues, so this isn't super urgent unless they're wrong.
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam Type-Bug
Removing security labels as null deref is a DoS and is not considered as a vulnerability as per https://chromium.googlesource.com/chromium/src/+/master/docs/security/faq.md#are-denial-of-service-issues-considered-security-bugs

Anyway, I guess that updating libpng wouldn't harm :)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5602fef50f7d262cfe960dca7a5be946daa8e347

commit 5602fef50f7d262cfe960dca7a5be946daa8e347
Author: Leon Scroggins III <scroggo@google.com>
Date: Thu Oct 05 16:17:57 2017

Update libpng to 1.6.34

Bug:  761608 
Bug:  771075 

Downloaded from http://libpng.download/src/libpng16

https://github.com/glennrp/libpng/commit/edef058e180be435afd8feebb742a22360a7fa3f
(included in this revision) moved the SSE opts directly into the main
libpng source, so contrib/intel/intel_sse.patch is no longer necessary,
as its changes are already included.

Other changes:
- README.chromium:
  - Update version number.
  - Remove references to cherry-picks from upstream, as the fixes are
    now included without cherry-picks.
  - Remove reference to applying intel_sse.patch, which is no longer
    necessary.
- BUILD.gn:
  - Build the intel code from its new location.
- PNGImageDecoderTest.cpp:
  - Remove unnecessary tests. TestFailureDuringDecode attempted to
    modify a valid PNG file to have an error, and then verified that it
    actually had an error, while leaving the FrameCount unchanged. I do
    not see much value to the test, and the apparently more lenient
    newer libpng no longer considers this modified file to be broken.

Change-Id: I2c7e517403609aed2051b91df47dd9f87f62e1ab
Reviewed-on: https://chromium-review.googlesource.com/671029
Reviewed-by: Leon Scroggins <scroggo@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Leon Scroggins <scroggo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506749}
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/BUILD.gn
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/LICENSE
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/README
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/README.chromium
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/arm/filter_neon.S
[delete] https://crrev.com/fe862f371864cd02c95f57152c4d9e6d6745cd11/third_party/libpng/contrib/intel/INSTALL
[delete] https://crrev.com/fe862f371864cd02c95f57152c4d9e6d6745cd11/third_party/libpng/contrib/intel/intel_sse.patch
[rename] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/intel/filter_sse2_intrinsics.c
[rename] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/intel/intel_init.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/png.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/png.h
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngconf.h
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngerror.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngget.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pnginfo.h
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngmem.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngpread.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngpriv.h
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngread.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngrio.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngrtran.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngrutil.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngset.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngstruct.h
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngtest.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngtrans.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngwio.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngwrite.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngwtran.c
[modify] https://crrev.com/5602fef50f7d262cfe960dca7a5be946daa8e347/third_party/libpng/pngwutil.c

Status: Fixed (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0dc539884fd4df44c4255fbf1f70b1a9df60a984

commit 0dc539884fd4df44c4255fbf1f70b1a9df60a984
Author: Leon Scroggins <scroggo@chromium.org>
Date: Fri Oct 06 13:15:04 2017

Revert "Update libpng to 1.6.34"

This reverts commit 5602fef50f7d262cfe960dca7a5be946daa8e347.

Reason for revert: Just to be extra cautious. Branch point is in one week; we can land afterwards to give it some more testing time. Will reland with the name mangling in https://chromium-review.googlesource.com/702694

Original change's description:
> Update libpng to 1.6.34
> 
> Bug:  761608 
> Bug:  771075 
> 
> Downloaded from http://libpng.download/src/libpng16
> 
> https://github.com/glennrp/libpng/commit/edef058e180be435afd8feebb742a22360a7fa3f
> (included in this revision) moved the SSE opts directly into the main
> libpng source, so contrib/intel/intel_sse.patch is no longer necessary,
> as its changes are already included.
> 
> Other changes:
> - README.chromium:
>   - Update version number.
>   - Remove references to cherry-picks from upstream, as the fixes are
>     now included without cherry-picks.
>   - Remove reference to applying intel_sse.patch, which is no longer
>     necessary.
> - BUILD.gn:
>   - Build the intel code from its new location.
> - PNGImageDecoderTest.cpp:
>   - Remove unnecessary tests. TestFailureDuringDecode attempted to
>     modify a valid PNG file to have an error, and then verified that it
>     actually had an error, while leaving the FrameCount unchanged. I do
>     not see much value to the test, and the apparently more lenient
>     newer libpng no longer considers this modified file to be broken.
> 
> Change-Id: I2c7e517403609aed2051b91df47dd9f87f62e1ab
> Reviewed-on: https://chromium-review.googlesource.com/671029
> Reviewed-by: Leon Scroggins <scroggo@chromium.org>
> Reviewed-by: Mike Klein <mtklein@chromium.org>
> Commit-Queue: Leon Scroggins <scroggo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506749}

TBR=scroggo@chromium.org,noel@chromium.org,mtklein@chromium.org

Change-Id: I75ef427e8dea83819d9dfdafa6bd714c9ab13539
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  761608 ,  771075 
Reviewed-on: https://chromium-review.googlesource.com/704954
Reviewed-by: Leon Scroggins <scroggo@chromium.org>
Commit-Queue: Leon Scroggins <scroggo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507041}
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/BUILD.gn
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/LICENSE
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/README
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/README.chromium
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/arm/filter_neon.S
[add] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/contrib/intel/INSTALL
[rename] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/contrib/intel/filter_sse2_intrinsics.c
[rename] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/contrib/intel/intel_init.c
[add] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/contrib/intel/intel_sse.patch
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/png.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/png.h
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngconf.h
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngerror.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngget.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pnginfo.h
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngmem.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngpread.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngpriv.h
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngread.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngrio.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngrtran.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngrutil.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngset.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngstruct.h
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngtest.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngtrans.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngwio.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngwrite.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngwtran.c
[modify] https://crrev.com/0dc539884fd4df44c4255fbf1f70b1a9df60a984/third_party/libpng/pngwutil.c

Mergedinto: 771075
Status: Duplicate (was: Fixed)
I removed the update to give it more time to bake (next release is in one week). Since this isn't a security issue, I'm going to just merge this into  issue 771075 , which tracks updating libpng.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f82653a473f8de5fc86d0f2ecc75f6237e61946b

commit f82653a473f8de5fc86d0f2ecc75f6237e61946b
Author: Leon Scroggins III <scroggo@google.com>
Date: Fri Oct 20 15:13:09 2017

Reland "Update libpng to 1.6.34"

This reverts commit 0dc539884fd4df44c4255fbf1f70b1a9df60a984.

We reverted the update just to be cautious. Now that the branch has
been cut, reland. Original message:
=======================================================================
Bug:  761608 
Bug:  771075 

Downloaded from http://libpng.download/src/libpng16

https://github.com/glennrp/libpng/commit/edef058e180be435afd8feebb742a22360a7fa3f
(included in this revision) moved the SSE opts directly into the main
libpng source, so contrib/intel/intel_sse.patch is no longer necessary,
as its changes are already included.

Other changes:
- README.chromium:
  - Update version number.
  - Remove references to cherry-picks from upstream, as the fixes are
    now included without cherry-picks.
  - Remove reference to applying intel_sse.patch, which is no longer
    necessary.
- BUILD.gn:
  - Build the intel code from its new location.
- PNGImageDecoderTest.cpp:
  - Remove unnecessary tests. TestFailureDuringDecode attempted to
    modify a valid PNG file to have an error, and then verified that it
    actually had an error, while leaving the FrameCount unchanged. I do
    not see much value to the test, and the apparently more lenient
    newer libpng no longer considers this modified file to be broken.
=======================================================================

Also includes name mangling for png_(get/set)_eXIf(_1) from
I11492d3e7cce34e7ba29b6399b45bec75b057948

Change-Id: Ie692badaf88403bc01b4bf52d95fe539f9079df5
Reviewed-on: https://chromium-review.googlesource.com/704209
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Leon Scroggins <scroggo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510438}
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/BUILD.gn
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/LICENSE
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/README
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/README.chromium
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/arm/filter_neon.S
[delete] https://crrev.com/2767edcdb266aea34a5fb4354a36d84858244fad/third_party/libpng/contrib/intel/INSTALL
[delete] https://crrev.com/2767edcdb266aea34a5fb4354a36d84858244fad/third_party/libpng/contrib/intel/intel_sse.patch
[rename] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/intel/filter_sse2_intrinsics.c
[rename] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/intel/intel_init.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/png.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/png.h
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngconf.h
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngerror.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngget.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pnginfo.h
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngmem.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngpread.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngprefix.h
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngpriv.h
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngread.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngrio.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngrtran.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngrutil.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngset.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngstruct.h
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngtest.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngtrans.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngwio.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngwrite.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngwtran.c
[modify] https://crrev.com/f82653a473f8de5fc86d0f2ecc75f6237e61946b/third_party/libpng/pngwutil.c

Sign in to add a comment