This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Hello Dan,

Clusterfuzz thinks this might be related to your CL https://pdfium.googlesource.com/pdfium.git/+/38fd84428a1ea007a043be0b7d9b289e47aa5da0.

Feel free to adjust labels or re-assign as appropriate.

Thanks!

The correct regression CL is: https://pdfium.googlesource.com/pdfium/+/cde5101eb15b24519e89fa500fe37038bc8e2201

The problem is, changing the use_count() check in fpdf_page_doc.cpp:334 to > 1 causes the code in fpdf_render_image to fail. We get a new CPDF_Object for the color space (count = 1) then create a CPDF_Colorspace object from it (count = 2) we then release the CPDF_Object (count = 1) and we, with the above change, free the colourspace and pCS references a deleted object.

Reverting the issue CL in order to fix.

The issue CL has been reverted so removing the release block label.

What wrong?

We call:
pCS  = LoadColorSpace(pCSObj) // this like new pCS.
RelaseColorSpace(pCSObj) // this like delete pCS .

// now pCS should be invalid.
Why after that pCS should be references a not deleted object ?

The problem is, that pCS is used after RelaseColorSpace() in 
https://cs.chromium.org/chromium/src/third_party/pdfium/core/fpdfapi/fpdf_render/fpdf_render_image.cpp?rcl=0&l=1034
In this case RelaseColorSpace() should be called after status.Initialize(...).

From the code, it's hard to tell which is the correct answer. If we should move the ReleaseColorSpace() call, or if pCS should not be used to access the family down on line 1042.

What different?
I think this is identical solution.

We can store family before.
Or
ReleaseColorSpace after.

----
Should I create CL?
And how I should it create:
One for this,
And another for revert of this revert?

My concern with the reverted CL is, how many other places do we have similar use-after-free issues once we change the ref counting? The original Cl that was reverted changed several classes ref counting, we should probably break that CL apart and land the change for each individual class. That way, if more of these come up we can just revert the bit that's necessary until the fix can be put into place.


We should land the fix for this separately regardless as it's a separate issue.

review URL:
https://codereview.chromium.org/2350193003/

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

commit 4b0e8cbc83b90f7343cf1a89dced6485e0711cff
Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org>
Date: Mon Sep 19 22:26:31 2016

Roll src/third_party/pdfium/ b12fbddec..c6c2e36f5 (3 commits).

https://pdfium.googlesource.com/pdfium.git/+log/b12fbddece43..c6c2e36f59f5

$ git log b12fbddec..c6c2e36f5 --date=short --no-merges --format='%ad %ae %s'
2016-09-19 tsepez Remove CPDF_Object::Destroy { delete this; }
2016-09-19 art-snake Fix "heap use after free" bug.
2016-09-19 kcwu Add fuzzer for fax codec

BUG= 647612 

TBR=dsinclair@chromium.org

Review-Url: https://codereview.chromium.org/2350993002
Cr-Commit-Position: refs/heads/master@{#419591}

[modify] https://crrev.com/4b0e8cbc83b90f7343cf1a89dced6485e0711cff/DEPS

 Issue 648576  has been merged into this issue.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.