Verified
Status Update
No update yet.
Comments
in...@chromium.org
Noel, can you please take a look.
no...@chromium.org
Had a quick look. Most I can say is that none of images decode at the JPEG level, they all fail during decoding.
I'm not sure why a <canvas> would accept undecoded images, but that seems to be involved. Adding Justin and Stephen for any advice on that aspect.
I'm not sure why a <canvas> would accept undecoded images, but that seems to be involved. Adding Justin and Stephen for any advice on that aspect.
lc...@google.com
They also occasionally produce variable bitmaps when just loaded on <img> (but yeah, djpeg doesn't decode them and they are messed up in a million ways - and they are a product of splicing other testcases generated earlier on, so I don't have a slightly different non-faulting image to compare to). Check this out:
http://lcamtuf.coredump.cx/chjpeg/foo.html
ke...@chromium.org
Justin or Stephen, can one of you please offer an opinion https://crbug.com/chromium/398235#c2 ?
I am putting security flags under the reasonably likely assumption that this is a renderer process memory leak.
I am putting security flags under the reasonably likely assumption that this is a renderer process memory leak.
no...@chromium.org
Image test case: http://lcamtuf.coredump.cx/chjpeg/foo.html
"Corrupt JPEG data: premature end of data segment" for all images. The image appears to have a valid header, but it's data areas have been truncated.
Changed JPEGDecoder::outputScanlines() to draw red pixels if it was ever called and re-loaded the test case. Saw 2 red images out of 300, and 4 accesses to the outputScanlines() (they occur in pairs)
-------- 0x3c9374a0 1 outputScanlines 0 (h=288)
-------- 0x3c9374a0 2 outputScanlines 0 (h=288)
-------- 0x3c937900 3 outputScanlines 0 (h=288)
-------- 0x3c937900 4 outputScanlines 0 (h=288)
Seems that the error recognition can happen after we have drawn decoded data to the image frame buffer sometimes, but what you are seeing on the page is decoded data in the frame buffer.
"Corrupt JPEG data: premature end of data segment" for all images. The image appears to have a valid header, but it's data areas have been truncated.
Changed JPEGDecoder::outputScanlines() to draw red pixels if it was ever called and re-loaded the test case. Saw 2 red images out of 300, and 4 accesses to the outputScanlines() (they occur in pairs)
-------- 0x3c9374a0 1 outputScanlines 0 (h=288)
-------- 0x3c9374a0 2 outputScanlines 0 (h=288)
-------- 0x3c937900 3 outputScanlines 0 (h=288)
-------- 0x3c937900 4 outputScanlines 0 (h=288)
Seems that the error recognition can happen after we have drawn decoded data to the image frame buffer sometimes, but what you are seeing on the page is decoded data in the frame buffer.
no...@chromium.org
Canvas case: http://lcamtuf.coredump.cx/chjpeg
"Corrupt JPEG data: premature end of data segment" for all images. Again a truncated image.
Again, changed JPEGDecoder::outputScanlines() to draw red pixels if it was ever called and reloaded the test case. Saw 5 red images out of 300, and 10 accesses to the outputScanlines() in pairs.
-------- 0x29330920 1 outputScanlines 0 (h=288)
-------- 0x29330920 2 outputScanlines 0 (h=288)
-------- 0x29332360 3 outputScanlines 0 (h=288)
-------- 0x29332360 4 outputScanlines 0 (h=288)
-------- 0x29336320 5 outputScanlines 0 (h=288)
-------- 0x29336320 6 outputScanlines 0 (h=288)
-------- 0x29336c80 7 outputScanlines 0 (h=288)
-------- 0x29336c80 8 outputScanlines 0 (h=288)
-------- 0x29337d60 9 outputScanlines 0 (h=288)
-------- 0x29337d60 10 outputScanlines 0 (h=288)
Same conclusion as above, you are seeing decoded data from the image frame buffer.
"Corrupt JPEG data: premature end of data segment" for all images. Again a truncated image.
Again, changed JPEGDecoder::outputScanlines() to draw red pixels if it was ever called and reloaded the test case. Saw 5 red images out of 300, and 10 accesses to the outputScanlines() in pairs.
-------- 0x29330920 1 outputScanlines 0 (h=288)
-------- 0x29330920 2 outputScanlines 0 (h=288)
-------- 0x29332360 3 outputScanlines 0 (h=288)
-------- 0x29332360 4 outputScanlines 0 (h=288)
-------- 0x29336320 5 outputScanlines 0 (h=288)
-------- 0x29336320 6 outputScanlines 0 (h=288)
-------- 0x29336c80 7 outputScanlines 0 (h=288)
-------- 0x29336c80 8 outputScanlines 0 (h=288)
-------- 0x29337d60 9 outputScanlines 0 (h=288)
-------- 0x29337d60 10 outputScanlines 0 (h=288)
Same conclusion as above, you are seeing decoded data from the image frame buffer.
no...@chromium.org
Image test case two: http://lcamtuf.coredump.cx/chjpeg/another.html
"Invalid SOS parameters for sequential JPEG" decode error for all 300 images loaded by that page.
Again, changed JPEGDecoder::outputScanlines() to draw red pixels, as before. This time 300 red images are drawn on the page, with 300 accesses to outputScanlines().
Error happens at a different place, but you are again seeing decoded data from the image frame buffer.
"Invalid SOS parameters for sequential JPEG" decode error for all 300 images loaded by that page.
Again, changed JPEGDecoder::outputScanlines() to draw red pixels, as before. This time 300 red images are drawn on the page, with 300 accesses to outputScanlines().
Error happens at a different place, but you are again seeing decoded data from the image frame buffer.
no...@chromium.org
So don't think this is security issue.
Perhaps if we want to draw the same thing regardless of how the error occurred in these cases, then a patch like the following is needed. I was unable to replicate the report with that patch applied.
Perhaps if we want to draw the same thing regardless of how the error occurred in these cases, then a patch like the following is needed. I was unable to replicate the report with that patch applied.
lc...@google.com
Why do you think this is not a security issue? The test case decodes to different images across multiple runs, so I'm guessing we're using unitialized memory at some point (and thus may end up leaking secrets).
cl...@chromium.org
[Comment Deleted]
ke...@chromium.org
[Empty comment from Monorail migration]
no...@chromium.org
> Why do you think this is not a security issue? The test case decodes to different images across multiple runs
I see decodes to the same pixels across multiple runs for these test cases:
Canvas test case:http://lcamtuf.coredump.cx/chjpeg
Image test case:http://lcamtuf.coredump.cx/chjpeg/foo.html
Here's the image test case result, where this time produced two decodes.
I see decodes to the same pixels across multiple runs for these test cases:
Canvas test case:
Image test case:
Here's the image test case result, where this time produced two decodes.
lc...@google.com
I'm getting 2-4 different bitmaps for <canvas>, although the PoCs aren't super-reliable - it sometimes takes multiple tries or a fresh profile. I can try to see what makes it more deterministic if you think that'll help.
lc...@google.com
This one seems a bit more reliable - always produces three distinct non-empty bitmaps on my Chrome install (dev) and also on canary:
http://lcamtuf.coredump.cx/chjpeg/index2a.html
no...@chromium.org
I was about to continue with :) ....
And sometimes I get just one decode, and the same pixels shown the attached image above.
>so I'm guessing we're using unitialized memory at some point
Strange that I get the same pixels each time.
Could you perhaps add a test-case to clusterfuzz? (clusterfuzz can detect uninitialized memory use I believe)
And sometimes I get just one decode, and the same pixels shown the attached image above.
>so I'm guessing we're using unitialized memory at some point
Strange that I get the same pixels each time.
Could you perhaps add a test-case to clusterfuzz? (clusterfuzz can detect uninitialized memory use I believe)
no...@chromium.org
I see "Corrupt JPEG data: premature end of data segment" errors for three test cases now.
http://lcamtuf.coredump.cx/chjpeg/
http://lcamtuf.coredump.cx/chjpeg/foo.html
http://lcamtuf.coredump.cx/chjpeg/index2a.html
#15 yeah, index2a.html is a bit more reliable. Possible to get a test-case like that in clusterfuzz?
#15 yeah, index2a.html is a bit more reliable. Possible to get a test-case like that in clusterfuzz?
lc...@google.com
No idea, inferno will know; I think they are using ASAN, which may be less thorough than Valgrind. Interestingly, index2a.html also repros in Firefox so it's probably libjpeg-related.
no...@chromium.org
No repo in Safari, and yeah index2a.html repros in Firefox me, and Chrome/Firefox use libjpeg_turbo.
no...@chromium.org
Got clusterfuzz to at least fetch and test your url, index2a.html.
mac-asan tested twice
-- no complaints.
windows-drmemory (valgrind)
-- dies complaining about leaked kernel handles in chrome's network layers (not helpful).
still waiting on linux-asan and windows_syzyasan results.
mac-asan tested twice
-- no complaints.
windows-drmemory (valgrind)
-- dies complaining about leaked kernel handles in chrome's network layers (not helpful).
still waiting on linux-asan and windows_syzyasan results.
no...@chromium.org
linux-asan & windows_syzyasan have no complaints either.
no...@chromium.org
Test runs for the record:
https://cluster-fuzz.appspot.com/testcase?key=6178066948685824
https://cluster-fuzz.appspot.com/testcase?key=5510327611424768
https://cluster-fuzz.appspot.com/testcase?key=5890666661937152
marked confirmed: the thing confirmed appears to be handle leaks in the chrome
network layer, perhaps because clusterfuzz forcibly shuts the test down.
https://cluster-fuzz.appspot.com/testcase?key=5027874740371456
https://cluster-fuzz.appspot.com/testcase?key=5732050872041472
marked confirmed: the thing confirmed appears to be handle leaks in the chrome
network layer, perhaps because clusterfuzz forcibly shuts the test down.
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
no...@chromium.org
@Justin, as an aside, noticed that if I did add a patch as in #9 say, then users of it could see a NULL return when calling BitmapImage->nativeImageForCurrentFrame() sometime after the image started decoding.
Most callers check the return code. I noticed that the new web api ImageBitmap stuff doesn't seem to NULL-check the nativeImageForCurrentFrame() return in places IIRC, and could crash chrome. Is that something worth fixing?
Most callers check the return code. I noticed that the new web api ImageBitmap stuff doesn't seem to NULL-check the nativeImageForCurrentFrame() return in places IIRC, and could crash chrome. Is that something worth fixing?
ke...@chromium.org
IIUC, ASan doesn't pick up uninitialized memory reads, but MSan (MemorySanitizer) does.
A query for open bugs with Cr-Internals-Skia and Stability-Memory-MemorySanitizer currently returns 3 results, as it happens.
A query for open bugs with Cr-Internals-Skia and Stability-Memory-MemorySanitizer currently returns 3 results, as it happens.
no...@chromium.org
The thing I love about the web most - urls and links.
no...@chromium.org
So #27, what am I missing?
ke...@chromium.org
Sorry, they are RestrictView-SecurityTeam, as this one is. I can cc you. This could be a duplicate of any of them, the best way to check would be to run the test case in an MSan-instrumented build.
no...@chromium.org
Thanks, looked at the issues you cc-ed to me and (sorry to say) no JPEG's in their stack traces: this bug is not a dup of any of them.
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Empty comment from Monorail migration]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Empty comment from Monorail migration]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
in...@chromium.org
No more M39 patches, moving to M40.
cl...@chromium.org
[Comment Deleted]
ti...@google.com
Hey noel@ - what's the latest here? Can you please provide an update on what needs to be done here? Thanks.
me...@chromium.org
I pinged noel@ offline.
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Empty comment from Monorail migration]
me...@chromium.org
[Empty comment from Monorail migration]
lc...@google.com
OK, better late than never. I believe I have a much simpler repro; this usually triggers two variants on my box, both in Chrome and in Firefox:
http://lcamtuf.coredump.cx/jpeg_simpler/
The original image:
http://lcamtuf.coredump.cx/jpeg_simpler/image_orig.jpg
Fault-triggering variant:
http://lcamtuf.coredump.cx/jpeg_simpler/image.jpg
The modification amounts to a single bit flip in byte 172:
ff c0 00 11 08 01 00 00 98 03 01 11 00 02 11 01
^^^^^ ^^^^^ ^^ ^^^^^ ^^^^^ ^^ ^^ ^^ ^^ ^^ ** ^^
SOF0 Lf P YRes XRes Nf Ci Hi/Vi Tqi Ci Hi/Vi Tqi
The byte being flipped is marked with **. It's the horizontal sampling factor for component 2. Between the cases, 0x11 becomes 0x31 - so Hi changes from 0x1 to 0x3.
However, I am not sure what's the significance of sampling factor 3, or why does it apparently make jpeg-turbo use uninitialized memory in the browser, without being reproducible with djpeg under ASAN / Valgrind.
Noel, does this help in any way?
The original image:
Fault-triggering variant:
The modification amounts to a single bit flip in byte 172:
ff c0 00 11 08 01 00 00 98 03 01 11 00 02 11 01
^^^^^ ^^^^^ ^^ ^^^^^ ^^^^^ ^^ ^^ ^^ ^^ ^^ ** ^^
SOF0 Lf P YRes XRes Nf Ci Hi/Vi Tqi Ci Hi/Vi Tqi
The byte being flipped is marked with **. It's the horizontal sampling factor for component 2. Between the cases, 0x11 becomes 0x31 - so Hi changes from 0x1 to 0x3.
However, I am not sure what's the significance of sampling factor 3, or why does it apparently make jpeg-turbo use uninitialized memory in the browser, without being reproducible with djpeg under ASAN / Valgrind.
Noel, does this help in any way?
lc...@google.com
Other potentially useful tidbits:
- Factors 2 and 4 are not affected, higher factors don't work,
- Changing horizontal and vertical sampling factors seems to have comparable results,
- This does not appear to be reproducible for sampling factors for component 1; but both chroma components (2 & 3) are affected in the same way.
- Factors 2 and 4 are not affected, higher factors don't work,
- Changing horizontal and vertical sampling factors seems to have comparable results,
- This does not appear to be reproducible for sampling factors for component 1; but both chroma components (2 & 3) are affected in the same way.
cl...@chromium.org
[Comment Deleted]
no...@chromium.org
#59> The modification amounts to a single bit flip in byte 172:
> ff c0 00 11 08 01 00 00 98 03 01 11 00 02 11 01
> ^^^^^ ^^^^^ ^^ ^^^^^ ^^^^^ ^^ ^^ ^^ ^^ ^^ ** ^^
> SOF0 Lf P YRes XRes Nf Ci Hi/Vi Tqi Ci Hi/Vi Tqi
> The byte being flipped is marked with **. It's the horizontal sampling factor for component 2. Between the > cases, 0x11 becomes 0x31 - so Hi changes from 0x1 to 0x3.
Looked at the spec:http://www.w3.org/Graphics/JPEG/itu-t81.pdf and 3 is allowed, but per http://dougkerr.net/Pumpkin/articles/Subsampling.pdf Figure 3, "Compressed JPEG Exif file subsampling encoding", 3 is never valid for a JPEG.
> However, I am not sure what's the significance of sampling factor 3, or why does it apparently make jpeg-turbo use uninitialized memory in the browser, without being reproducible with djpeg under ASAN / Valgrind.
Dunno why is does not reproduce in djpeg (assume you mean the libjpeg_turbo one).
Noel, does this help in any way?
> ff c0 00 11 08 01 00 00 98 03 01 11 00 02 11 01
> ^^^^^ ^^^^^ ^^ ^^^^^ ^^^^^ ^^ ^^ ^^ ^^ ^^ ** ^^
> SOF0 Lf P YRes XRes Nf Ci Hi/Vi Tqi Ci Hi/Vi Tqi
> The byte being flipped is marked with **. It's the horizontal sampling factor for component 2. Between the > cases, 0x11 becomes 0x31 - so Hi changes from 0x1 to 0x3.
Looked at the spec:
> However, I am not sure what's the significance of sampling factor 3, or why does it apparently make jpeg-turbo use uninitialized memory in the browser, without being reproducible with djpeg under ASAN / Valgrind.
Dunno why is does not reproduce in djpeg (assume you mean the libjpeg_turbo one).
Noel, does this help in any way?
no...@chromium.org
> #60
> - Factors 2 and 4 are not affected, higher factors don't work,
Seems we should detect a 3 then...
> - Changing horizontal and vertical sampling factors seems to have comparable results,
in either horizontal and vertical sampling factors
- This does not appear to be reproducible for sampling factors for component 1; but both chroma components (2 & 3) are affected in the same way.
even the first for simplicity, since 3 is never valid for any of the components.
> - Factors 2 and 4 are not affected, higher factors don't work,
Seems we should detect a 3 then...
> - Changing horizontal and vertical sampling factors seems to have comparable results,
in either horizontal and vertical sampling factors
- This does not appear to be reproducible for sampling factors for component 1; but both chroma components (2 & 3) are affected in the same way.
even the first for simplicity, since 3 is never valid for any of the components.
no...@chromium.org
> #60 Other potentially useful tidbits:
> - Factors 2 and 4 are not affected, higher factors don't work,
lcamtuf@ "Factors 2 and 4 are not affected" good, thanks. Did you happen to test Factor 1? The current fix reset any Factor 3 to Factor 1. Another possible fix is deem Factor 3 JPEG's invalid.
> - Factors 2 and 4 are not affected, higher factors don't work,
lcamtuf@ "Factors 2 and 4 are not affected" good, thanks. Did you happen to test Factor 1? The current fix reset any Factor 3 to Factor 1. Another possible fix is deem Factor 3 JPEG's invalid.
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
cl...@chromium.org
[Comment Deleted]
no...@chromium.org
Removed the nags from CF. We are yet to reproduce any of this bug on any CF bot. I added the fault image in #59 to the CF. Also sent off a CF URL fetch test of http://lcamtuf.coredump.cx/jpeg_simpler/image.jpg ...
no...@chromium.org
Widened the URL fetch test on CF to win/mac/linux asan & msan bots. The results look fine to me.
+earthdoc@ If there is some memory issue as Michal suspects (#59), the CF msan bots are not seeing / reporting it.
+earthdoc@ If there is some memory issue as Michal suspects (#59), the CF msan bots are not seeing / reporting it.
no...@chromium.org
Meanwhile, we have a fix think, awaiting feedback on #65 to help land it.
[Deleted User] <[Deleted User]>
MSan builds disable assembly optimizations in libjpeg_turbo:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjpeg_turbo/libjpeg.gyp&l=139
This bug may be specific to the asm implementation, though I would expect Valgrind to still pick it up in that case.
#20 - DrMemory is actually unrelated to Valgrind.
This bug may be specific to the asm implementation, though I would expect Valgrind to still pick it up in that case.
#20 - DrMemory is actually unrelated to Valgrind.
no...@chromium.org
No pick up so far for the URL tests. Another approach: grab http://lcamtuf.coredump.cx/jpeg_simpler/ and wrap it in a Blink http layout test. Mark the test [ Skip ] in TestExpectations... CF ignores [ Skip ] and runs the test regardless, right?
Add the three images seen on this bug to the test. Make the test select one and load / decode it 300 times (Michal's code)... and see if the CF bots (asm or not) complain.
https://codereview.chromium.org/1048003002
reviewers: r?
Add the three images seen on this bug to the test. Make the test select one and load / decode it 300 times (Michal's code)... and see if the CF bots (asm or not) complain.
reviewers: r?
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=192900
------------------------------------------------------------------
r192900 | noel@chromium.org | 2015-04-01T03:32:38.658144Z
Changed paths:
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/multiple-jpeg-decodes.html?r1=192900&r2=192899&pathrev=192900
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources?r1=192900&r2=192899&pathrev=192900
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-000.jpg?r1=192900&r2=192899&pathrev=192900
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-001.jpg?r1=192900&r2=192899&pathrev=192900
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-002.jpg?r1=192900&r2=192899&pathrev=192900
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/multiple-jpeg-decodes-expected.txt?r1=192900&r2=192899&pathrev=192900
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=192900&r2=192899&pathrev=192900
Add a multiple image decode http layout test
Not enabled for run-webkit-tests (marked [Skip] in TestExpectations) but
used in helper systems that wrap run-webkit-tests. Test is dumpAsText(),
which allows random selection of the test image. Images are served after
a short pause to jitter network packet delivery.
TBR=inferno@chromium.org
BUG=398235
NOTRY=true
Review URL:https://codereview.chromium.org/1048003002
-----------------------------------------------------------------
------------------------------------------------------------------
r192900 | noel@chromium.org | 2015-04-01T03:32:38.658144Z
Changed paths:
A
A
A
A
A
A
M
Add a multiple image decode http layout test
Not enabled for run-webkit-tests (marked [Skip] in TestExpectations) but
used in helper systems that wrap run-webkit-tests. Test is dumpAsText(),
which allows random selection of the test image. Images are served after
a short pause to jitter network packet delivery.
TBR=inferno@chromium.org
BUG=398235
NOTRY=true
Review URL:
-----------------------------------------------------------------
no...@chromium.org
[Empty comment from Monorail migration]
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=192980
------------------------------------------------------------------
r192980 | noel@chromium.org | 2015-04-02T01:44:46.479913Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp?r1=192980&r2=192979&pathrev=192980
Add helper to validate JPEG subsampling factors
Use it to turn the invalid horizontal/vertical subsampling factor 3 to a
1, which is fine for both the YUV decoding path (it won't turn on if any
of the component subsamplings is 3) and the normal JPEG decode path.
TEST=Covered by the test added in r192900
BUG=398235
Review URL:https://codereview.chromium.org/1039503003
-----------------------------------------------------------------
------------------------------------------------------------------
r192980 | noel@chromium.org | 2015-04-02T01:44:46.479913Z
Changed paths:
M
Add helper to validate JPEG subsampling factors
Use it to turn the invalid horizontal/vertical subsampling factor 3 to a
1, which is fine for both the YUV decoding path (it won't turn on if any
of the component subsamplings is 3) and the normal JPEG decode path.
TEST=Covered by the test added in r192900
BUG=398235
Review URL:
-----------------------------------------------------------------
no...@chromium.org
CF now has stuff to chew on to vet this issue.
@#59 You reproduced on Mozilla right?
@#59 You reproduced on Mozilla right?
Michal, maybe file a Mozilla bug about this particular case? cc-me noel.gordon at gmail. Moz fuzzing infra differs from ours and might spot something we don't. I can work with Seth (image core owner at Mozilla) to get the libjpeg_turbo maintainer into the discussion if needed.
lc...@google.com
I have a bug with Mozilla (about as ancient as this one :-) Will cc: you.
cl...@chromium.org
[Empty comment from Monorail migration]
pd...@chromium.org
[Empty comment from Monorail migration]
no...@chromium.org
Thanks for the Moz bug access. As I have shown therein, this issue is due to the way the network delivers packets of data to JPEG decoder, and that multiple variants are produced depending on data packet size.
I also tested with libjpeg6b and found it 1) accepts the Factor 3 fault image fine and 2) always produces the same rendering of that image no matter how the network delivers the data (chunk size).
Since libjpeg6b accepts the Factor 3 image, we should, and so I will back out the Factor 3 restriction I added in #77. Also from the mozilla bug report, another test case was:
http://lcamtuf.coredump.cx/ffjpeg/
and I noted Chrome produces to multiple variants for that case too. So I'm adding the image from test case to test corpus of images we have for this issue (see #75) and adjusting the layout test to use it.
https://codereview.chromium.org/1067243003 uploaded.
I also tested with libjpeg6b and found it 1) accepts the Factor 3 fault image fine and 2) always produces the same rendering of that image no matter how the network delivers the data (chunk size).
Since libjpeg6b accepts the Factor 3 image, we should, and so I will back out the Factor 3 restriction I added in #77. Also from the mozilla bug report, another test case was:
and I noted Chrome produces to multiple variants for that case too. So I'm adding the image from test case to test corpus of images we have for this issue (see #75) and adjusting the layout test to use it.
no...@chromium.org
@pdr ^^^ this should be enough to move forward here, right?
pd...@chromium.org
That sounds reasonable to me, but I'm not familiar with libjpeg6b. I've added pkasting and urvang to help with the review of https://codereview.chromium.org/1067243003 .
lc...@google.com
The change in #82 looks OK to me (and looks like Noel narrowed the bad code down to jdhuff.c)
no...@chromium.org
#85 Thanks Michal, makes sense to me... net net with the change I've just posted we have added a layout test for this bug to chrome so far, and that is all we changed.
The curious can add themselves tohttps://bugzilla.mozilla.org/show_bug.cgi?id=1050342 to follow along to see the nail going into this apparently very difficult to understand bug.
The curious can add themselves to
ti...@google.com
Moving from M42 --> M43.
no...@chromium.org
#86 > That sounds reasonable to me, but I'm not familiar with libjpeg6b. I've added pkasting and urvang to help with the review of https://codereview.chromium.org/1067243003 .
No need to be intimately familiar with libjpeg6b except for one fact: libjpeg_turbo should produce the exact same decoded image, byte-for-byte, as libjpeg6b in all circumstances, for the same input encoded image data, this being the stated goal of the libjpeg_turbo project.
Michal has provided 4 images, each with a test-case, showing that that project goal has been broken, that libjpeg_turbo is producing multiple decoded image outputs (variants) for the same input data. I have shown that this bug manifests in libjpeg_turbo only when the input data is delivered via network packets, and that libjpeg6b does not exhibit the bug in any circumstance (network, no network) for any of the 4 images.
So I think for now we should back-out the code restriction I added for Factor 3 images since such images a acceptable in libjpeg6b (sugoi@ can review at leisure), and also widen the layout test case (which is run in ClusterFuzz only) with the new fault image fromhttp://lcamtuf.coredump.cx/ffjpeg
No need to be intimately familiar with libjpeg6b except for one fact: libjpeg_turbo should produce the exact same decoded image, byte-for-byte, as libjpeg6b in all circumstances, for the same input encoded image data, this being the stated goal of the libjpeg_turbo project.
Michal has provided 4 images, each with a test-case, showing that that project goal has been broken, that libjpeg_turbo is producing multiple decoded image outputs (variants) for the same input data. I have shown that this bug manifests in libjpeg_turbo only when the input data is delivered via network packets, and that libjpeg6b does not exhibit the bug in any circumstance (network, no network) for any of the 4 images.
So I think for now we should back-out the code restriction I added for Factor 3 images since such images a acceptable in libjpeg6b (sugoi@ can review at leisure), and also widen the layout test case (which is run in ClusterFuzz only) with the new fault image from
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=193326
------------------------------------------------------------------
r193326 | noel@chromium.org | 2015-04-08T05:13:34.360217Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp?r1=193326&r2=193325&pathrev=193326
Revert of Add helper to validate JPEG decode subsampling factors (patchset #1 id:1 ofhttps://codereview.chromium.org/1039503003/ )
Reason for revert:
Per the bug, Factor 3 images are acceptable to libjpeg6b and users can even create them using its well-known cjpeg tool.
Original issue's description:
------------------------------------------------------------------
r193326 | noel@chromium.org | 2015-04-08T05:13:34.360217Z
Changed paths:
M
Revert of Add helper to validate JPEG decode subsampling factors (patchset #1 id:1 of
Reason for revert:
Per the bug, Factor 3 images are acceptable to libjpeg6b and users can even create them using its well-known cjpeg tool.
Original issue's description:
TBR=sugoi@chromium.org,reveman@chromium.org,pdr@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=398235
Review URL:
-----------------------------------------------------------------
no...@chromium.org
"Actually, 3:1:1 video does exist in Sony HDCAMs, so a chroma subsampling factor of 3 is not unheard-of. The libjpeg API was created to be very flexible in this regard, and there is nothing to prevent one from generating a JPEG file in which the luma is subsampled instead of the chroma.
cjpeg -sample 1x1,3x1,3x1
will generate such a "1:3:3" JPEG file, which is extremely unorthodox and probably even stupid, but it's currently displayable by every browser on the planet."
Created a jpeg to test that statement, seems so (factor3.jpg attached)
% ./djpeg -debug -outfile /dev/null factor3.jpg
Independent JPEG Group's DJPEG, version 6b 27-Mar-1998
Copyright (C) 1998, Thomas G. Lane
read input packet size: 4096
Start of Image
JFIF APP0 marker: version 1.01, density 1x1 0
Define Quantization Table 0 precision 0
Define Quantization Table 1 precision 0
Start Of Frame 0xc0: width=136, height=51, components=3
Component 1: 1hx1v q=0
Component 2: 3hx1v q=1 <- factor 3
Component 3: 3hx1v q=1 <- factor 3
Define Huffman Table 0x00
Define Huffman Table 0x10
Define Huffman Table 0x01
Define Huffman Table 0x11
Start Of Scan: 3 components
Component 1: dc=0 ac=0
Component 2: dc=1 ac=1
Component 3: dc=1 ac=1
Ss=0, Se=63, Ah=0, Al=0
read input packet size: 2247
End Of Image
no...@chromium.org
@earthdok #73
> MSan builds disable assembly optimizations in libjpeg_turbo:
>https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjpeg_turbo/libjpeg.gyp&l=139
> This bug may be specific to the asm implementation, though I would expect Valgrind to still pick it up in that case.
I've built local copies of libjpeg_turbo trunk 1.4.80 on OSX --with-simd (asm) and --without-simd (no asm) under it's ./configure options, and modified its djpeg.c tool to deliver the input data in packets (to simulate the network). The bug reproduces in both cases.
> MSan builds disable assembly optimizations in libjpeg_turbo:
>
> This bug may be specific to the asm implementation, though I would expect Valgrind to still pick it up in that case.
I've built local copies of libjpeg_turbo trunk 1.4.80 on OSX --with-simd (asm) and --without-simd (no asm) under it's ./configure options, and modified its djpeg.c tool to deliver the input data in packets (to simulate the network). The bug reproduces in both cases.
no...@chromium.org
>Michal has provided 4 images, each with a test-case, showing that that project goal has been broken, that libjpeg_turbo is producing multiple decoded image outputs (variants) for the same input data.
Attaching the 4 test images here, will upload them also to CF.
Attaching the 4 test images here, will upload them also to CF.
no...@chromium.org
Done. Added factor3.jpg and 4 others with Factor 3 samplings (atom-sampling-1-3-1.jpg atom-sampling-1-3-3.jpg atom-sampling-3-1-1.jpg atom-sampling-3-3-1.jpg) for good measure.
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=193374
------------------------------------------------------------------
r193374 | noel@chromium.org | 2015-04-08T18:16:04.566130Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/multiple-jpeg-decodes.html?r1=193374&r2=193373&pathrev=193374
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-003.jpg?r1=193374&r2=193373&pathrev=193374
Multiple image decode http layout test: add another test image
Add new image jpeg-image-003.jpg, and adjust the test to randomly select
from the (now) 4 jpeg images in the test corpus.
TEST=http/tests/images/multiple-jpeg-decodes.html
BUG=398235
Review URL:https://codereview.chromium.org/1064353003
-----------------------------------------------------------------
------------------------------------------------------------------
r193374 | noel@chromium.org | 2015-04-08T18:16:04.566130Z
Changed paths:
M
A
Multiple image decode http layout test: add another test image
Add new image jpeg-image-003.jpg, and adjust the test to randomly select
from the (now) 4 jpeg images in the test corpus.
TEST=http/tests/images/multiple-jpeg-decodes.html
BUG=398235
Review URL:
-----------------------------------------------------------------
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194484
------------------------------------------------------------------
r194484 | noel@chromium.org | 2015-04-27T05:43:03.146332Z
Changed paths:
Dhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-001.jpg?r1=194484&r2=194483&pathrev=194484
Dhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-002.jpg?r1=194484&r2=194483&pathrev=194484
Dhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/multiple-jpeg-decodes-expected.txt?r1=194484&r2=194483&pathrev=194484
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=194484&r2=194483&pathrev=194484
Dhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/multiple-jpeg-decodes.html?r1=194484&r2=194483&pathrev=194484
Dhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-000.jpg?r1=194484&r2=194483&pathrev=194484
Revert of Add a multiple image decode http layout test (patchset #2 id:20001 ofhttps://codereview.chromium.org/1048003002/ )
Reason for revert:
Helper systems detected nothing: remove this test.
Original issue's description:
------------------------------------------------------------------
r194484 | noel@chromium.org | 2015-04-27T05:43:03.146332Z
Changed paths:
D
D
D
M
D
D
Revert of Add a multiple image decode http layout test (patchset #2 id:20001 of
Reason for revert:
Helper systems detected nothing: remove this test.
Original issue's description:
TBR=inferno@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=398235
Review URL:
-----------------------------------------------------------------
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194486
------------------------------------------------------------------
r194486 | noel@chromium.org | 2015-04-27T06:07:03.789506Z
Changed paths:
Dhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/images/resources/jpeg-image-003.jpg?r1=194486&r2=194485&pathrev=194486
Remove Multiple image decode http layout test images
The multiple impage decode test was removed; here remove the remaining
image associated with that test.
TBR=urvang@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=398235
Review URL:https://codereview.chromium.org/1100143003
-----------------------------------------------------------------
------------------------------------------------------------------
r194486 | noel@chromium.org | 2015-04-27T06:07:03.789506Z
Changed paths:
D
Remove Multiple image decode http layout test images
The multiple impage decode test was removed; here remove the remaining
image associated with that test.
TBR=urvang@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=398235
Review URL:
-----------------------------------------------------------------
no...@chromium.org
Backed out the tests. My analysis of jdhuff.c tells me this is not a security bug, just a loss in libjpeg6b compat caused by libjpeg_turbo svn r64, which produces different renderings depending on the size of the network packet data sent to the decoder (refer to the mozilla bug). Will change sec-labels when all done here.
bu...@chromium.org
------------------------------------------------------------------
r295003 | noel@chromium.org | 2015-04-27T11:27:28.040505Z
Changed paths:
Mhttp://src.chromium.org/viewvc/chrome/trunk/deps/third_party/libjpeg_turbo/google.patch?r1=295003&r2=295002&pathrev=295003
Mhttp://src.chromium.org/viewvc/chrome/trunk/deps/third_party/libjpeg_turbo/jdhuff.h?r1=295003&r2=295002&pathrev=295003
Mhttp://src.chromium.org/viewvc/chrome/trunk/deps/third_party/libjpeg_turbo/README.chromium?r1=295003&r2=295002&pathrev=295003
Mhttp://src.chromium.org/viewvc/chrome/trunk/deps/third_party/libjpeg_turbo/jdhuff.c?r1=295003&r2=295002&pathrev=295003
Fix libjpeg_turbo svn r64 libjpeg6b compat issue
Make the fast path Huffman decoder fallback to the slow decoding path if
the Huffman decoding bit sentinel > 16, this to match libjpeg6b decoding
by reproducing the exact behavior of jpeg_huff_decode().
When this sentinel check is missing (see bug), libjpeg_turbo can produce
two (or more) different decoded images for the same input data depending
on how transport systems (eg., networks) packetize the data for delivery
to end-systems, web browsers for example.
Note: libjpeg6b produces the same decoded image, irrespective of how the
input data is placed in network packets. This fix makes libjpeg_turbo do
the same for compat with libjpeg6b.
TBR=darin@chromium.org
BUG=chromium:398235
Review URL:https://codereview.appspot.com/229430043
-----------------------------------------------------------------
r295003 | noel@chromium.org | 2015-04-27T11:27:28.040505Z
Changed paths:
M
M
M
M
Fix libjpeg_turbo svn r64 libjpeg6b compat issue
Make the fast path Huffman decoder fallback to the slow decoding path if
the Huffman decoding bit sentinel > 16, this to match libjpeg6b decoding
by reproducing the exact behavior of jpeg_huff_decode().
When this sentinel check is missing (see bug), libjpeg_turbo can produce
two (or more) different decoded images for the same input data depending
on how transport systems (eg., networks) packetize the data for delivery
to end-systems, web browsers for example.
Note: libjpeg6b produces the same decoded image, irrespective of how the
input data is placed in network packets. This fix makes libjpeg_turbo do
the same for compat with libjpeg6b.
TBR=darin@chromium.org
BUG=chromium:398235
Review URL:
-----------------------------------------------------------------
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194511
------------------------------------------------------------------
r194511 | noel@chromium.org | 2015-04-27T12:42:00.088603Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=194511&r2=194510&pathrev=194511
Mark fast/images/2-dht.html TestExpectations [ ImageOnlyFailure ]
The test fast/images/2-dht.html will need new image rebaselines once the
fix forhttps://crbug.com/chromium/398235 is submitted. Mark the test as an image failure for
now, while we roll the fixhttps://codereview.appspot.com/229430043 into
the chromium tree.
TBR=urvang@chromium.org
NOTRY=true
BUG=398235
Review URL:https://codereview.chromium.org/1103143003
-----------------------------------------------------------------
------------------------------------------------------------------
r194511 | noel@chromium.org | 2015-04-27T12:42:00.088603Z
Changed paths:
M
Mark fast/images/2-dht.html TestExpectations [ ImageOnlyFailure ]
The test fast/images/2-dht.html will need new image rebaselines once the
fix for
now, while we roll the fix
the chromium tree.
TBR=urvang@chromium.org
NOTRY=true
BUG=398235
Review URL:
-----------------------------------------------------------------
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194629
------------------------------------------------------------------
r194629 | noel@chromium.org | 2015-04-28T22:53:11.023190Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=194629&r2=194628&pathrev=194629
Mark virtual fast/images/2-dht.html tests as [ ImageOnlyFailure ]
virtual/{deferred,slimmingpaint}/fast/images/2-dht.html need a new image
rebaseline for the fix forhttps://crbug.com/chromium/398235 to be submitted. Make them image
only failures in TestExpectations (following blink r194511).
virtual/slimmingpaint/fast/images/2-dht.html [ ImageOnlyFailure ]
virtual/deferred/fast/images/2-dht.html [ ImageOnlyFailure ]
NOTRY=true
BUG=398235
Review URL:https://codereview.chromium.org/1103273005
-----------------------------------------------------------------
------------------------------------------------------------------
r194629 | noel@chromium.org | 2015-04-28T22:53:11.023190Z
Changed paths:
M
Mark virtual fast/images/2-dht.html tests as [ ImageOnlyFailure ]
virtual/{deferred,slimmingpaint}/fast/images/2-dht.html need a new image
rebaseline for the fix for
only failures in TestExpectations (following blink r194511).
virtual/slimmingpaint/fast/images/2-dht.html [ ImageOnlyFailure ]
virtual/deferred/fast/images/2-dht.html [ ImageOnlyFailure ]
NOTRY=true
BUG=398235
Review URL:
-----------------------------------------------------------------
no...@chromium.org
+scroggo (Leon) as an FYI re: what's involved in the current two-sided-patch dance for updating image decoders, #98 onwards.
no...@chromium.org
HUFF_DECODE_FAST security:
libjpeg_turbo svn r64 also causedhttps://crbug.com/chromium/299835 (CVE-2013-6630), for the record, by fiddling with Huffman decoder table access. Some of the multiple image renderings seen in this bug are a side-effect of r64 (fixed with #98), again due to invalid Huffman table access in the fast path Huffman decoder, macro HUFF_DECODE_FAST. However, these tables accesses are always within table range (not a security issue), but can produce two or more image renderings for the same input data depending on how the network delivers compressed image packets to the JPEG decoder, a compat issue only.
FILL_BIT_BUFFER_FAST security:
r64 added another macro: FILL_BIT_BUFFER_FAST. This macro reads the input data stream, unchecked for the bounds of the input stream. Conjecture: JPEG byte stuffing and/or maximal length Huffman codeword sequences in the input might convince that macro to read past the input buffer length.
The only input data read protection is during Huffman decoding mode selection (fast vs. slow path decoding) in decode_mcu(),https://github.com/mozilla/mozjpeg/blob/master/jdhuff.c#L763
if (cinfo->src->bytes_in_buffer < BUFSIZE * (size_t)cinfo->blocks_in_MCU
|| cinfo->unread_marker != 0)
usefast = 0;
which disables fast path decoding when there are too few input bytes. BUFSIZE is 128, and info->blocks_in_MCU = 1 for a grayscale JPEG, so fast path decoding is disabled at least < 128 bytes from the end of the input data stream. So by having exactly 128 bytes at the end of the compressed image, the fast path can be activated. My various attempts at appending maximal length codeword sequences and/or 0xFF00 byte stuffed data into the last 128 bytes of the input stream of a grayscale JPEG image did make the fast path read, but only 96 bytes of input at most. I have no evidence at this time that FILL_BIT_BUFFER_FAST is unsafe (reads past the input data buffer).
libjpeg_turbo svn r64 also caused
FILL_BIT_BUFFER_FAST security:
r64 added another macro: FILL_BIT_BUFFER_FAST. This macro reads the input data stream, unchecked for the bounds of the input stream. Conjecture: JPEG byte stuffing and/or maximal length Huffman codeword sequences in the input might convince that macro to read past the input buffer length.
The only input data read protection is during Huffman decoding mode selection (fast vs. slow path decoding) in decode_mcu(),
if (cinfo->src->bytes_in_buffer < BUFSIZE * (size_t)cinfo->blocks_in_MCU
|| cinfo->unread_marker != 0)
usefast = 0;
which disables fast path decoding when there are too few input bytes. BUFSIZE is 128, and info->blocks_in_MCU = 1 for a grayscale JPEG, so fast path decoding is disabled at least < 128 bytes from the end of the input data stream. So by having exactly 128 bytes at the end of the compressed image, the fast path can be activated. My various attempts at appending maximal length codeword sequences and/or 0xFF00 byte stuffed data into the last 128 bytes of the input stream of a grayscale JPEG image did make the fast path read, but only 96 bytes of input at most. I have no evidence at this time that FILL_BIT_BUFFER_FAST is unsafe (reads past the input data buffer).
no...@chromium.org
Perf: how it's done at libjpeg_turbo is an unknown. I asked about perf testing on the moz bug and was referred to http://www.libjpeg-turbo.org/About/Performance
A wider, more varied corpus of images is needed to perf test JPEG in a browser. I tested on 4 machines, comparing fast-huff (what we had) to fixed-huff (#98 r295003). The change is about neutral, but costs 4 days of work to measure.
win7-t530-libjpeg-turbo-500-decode fast-huff vs. slow-huff vs... jpeg-142-corpus
win-z600-turbo-500-decode fast-huff vs. slow-huff vs... jpeg-142-corpus
macbook-air-libjpeg-turbo-500-decode fast-huff vs. slow-huff vs... jpeg-142-corpus
macpro-libjpeg-turbo-500-decode fast-huff vs. slow-huff vs... jpeg-142-corpus
no...@chromium.org
Results look ok, rolling the change into chrome.
bu...@chromium.org
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/c038962a97c41089b6ef09f498bfd934be95b5cc
commit c038962a97c41089b6ef09f498bfd934be95b5cc
Author: noel <noel@chromium.org>
Date: Wed Apr 29 14:03:43 2015
Roll libjpeg_turbo to r295003 (git 8ee9bdd068eff)
Roll in r295003: "Fix libjpeg_turbo svn r64 libjpeg6b compat issue" from
https://codereview.appspot.com/229430043/
TBR=fdegans@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel,mac_blink_rel,win_blink_rel
BUG=398235
Review URL:https://codereview.chromium.org/1110113002
Cr-Commit-Position: refs/heads/master@{#327484}
[modify]http://crrev.com/c038962a97c41089b6ef09f498bfd934be95b5cc/DEPS
commit c038962a97c41089b6ef09f498bfd934be95b5cc
Author: noel <noel@chromium.org>
Date: Wed Apr 29 14:03:43 2015
Roll libjpeg_turbo to r295003 (git 8ee9bdd068eff)
Roll in r295003: "Fix libjpeg_turbo svn r64 libjpeg6b compat issue" from
TBR=fdegans@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel,mac_blink_rel,win_blink_rel
BUG=398235
Review URL:
Cr-Commit-Position: refs/heads/master@{#327484}
[modify]
no...@chromium.org
One image in #92, jpeg-image-002.jpg, does not have mutliple variants. Instead, it is either drawn or not drawn, which is different issue. A rendering bug maybe, but there is no compat drawing that image in any browser (garbage in, garbage out) and there is no security issue with that image that I could find.
cl...@chromium.org
Adding Merge-Triage label for tracking purposes.
Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.
When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information onomahaproxy.appspot.com .
- Your friendly ClusterFuzz
Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.
When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on
- Your friendly ClusterFuzz
no...@chromium.org
No merge needed: the fix can bake in normal dev->beta->stable release flow.
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194689
------------------------------------------------------------------
r194689 | noel@chromium.org | 2015-04-30T01:32:08.226624Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=194689&r2=194688&pathrev=194689
Rebaseline fast/images/2-dht.html image results
The libjpeg_turbo svn r64 bug fix has rolled into the Chromium tree. Now
rebaseline the image results of the associated layout test.
TBR=urvang@chromium.org
NOTRY=true
BUG=398235
Review URL:https://codereview.chromium.org/1110653004
-----------------------------------------------------------------
------------------------------------------------------------------
r194689 | noel@chromium.org | 2015-04-30T01:32:08.226624Z
Changed paths:
M
Rebaseline fast/images/2-dht.html image results
The libjpeg_turbo svn r64 bug fix has rolled into the Chromium tree. Now
rebaseline the image results of the associated layout test.
TBR=urvang@chromium.org
NOTRY=true
BUG=398235
Review URL:
-----------------------------------------------------------------
no...@chromium.org
rebaseline-o-matic is stalled, mailed chrome-troopers.
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194756
------------------------------------------------------------------
r194756 | pdr@chromium.org | 2015-04-30T17:31:44.500439Z
Changed paths:
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=194756&r2=194755&pathrev=194756
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/android/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/android/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win-xp/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win-xp/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/android/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Ahttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win-xp/virtual/deferred/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/virtual/slimmingpaint/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/fast/images/2-dht-expected.png?r1=194756&r2=194755&pathrev=194756
Auto-rebaseline for r194689
http://src.chromium.org/viewvc/blink?view=revision&revision=194689
BUG=398235
TBR=noel@chromium.org
Review URL:https://codereview.chromium.org/1121483002
-----------------------------------------------------------------
------------------------------------------------------------------
r194756 | pdr@chromium.org | 2015-04-30T17:31:44.500439Z
Changed paths:
A
M
M
M
M
A
M
M
M
M
M
M
M
A
A
M
A
A
A
M
M
M
Auto-rebaseline for r194689
BUG=398235
TBR=noel@chromium.org
Review URL:
-----------------------------------------------------------------
no...@chromium.org
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=194776
------------------------------------------------------------------
r194776 | noel@chromium.org | 2015-04-30T22:08:15.054425Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=194776&r2=194775&pathrev=194776
Recover fast/images/2-dht.html [ Crash Pass ] expectations on [ Mac ]
Was removed to land the fix for 398235. Recover Crash expectation on Mac
perhttps://crbug.com/chromium/474168 .
BUG=398235,474168
NOTRY=true
Review URL:https://codereview.chromium.org/1121603002
-----------------------------------------------------------------
------------------------------------------------------------------
r194776 | noel@chromium.org | 2015-04-30T22:08:15.054425Z
Changed paths:
M
Recover fast/images/2-dht.html [ Crash Pass ] expectations on [ Mac ]
Was removed to land the fix for 398235. Recover Crash expectation on Mac
per
BUG=398235,474168
NOTRY=true
Review URL:
-----------------------------------------------------------------
no...@chromium.org
[Comment Deleted]
ti...@google.com
Based on #108, rolling in with M-44. Thanks Noel for all your work here!
la...@google.com
[Empty comment from Monorail migration]
bu...@chromium.org
The following revision refers to this bug:
http://src.chromium.org/viewvc/blink?view=rev&rev=195776
------------------------------------------------------------------
r195776 | noel@chromium.org | 2015-05-22T09:04:56.047528Z
Changed paths:
Mhttp://src.chromium.org/viewvc/blink/branches/chromium/2357/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp?r1=195776&r2=195775&pathrev=195776
Merge 193326 "Revert of Add helper to validate JPEG decode subsa..."
------------------------------------------------------------------
r195776 | noel@chromium.org | 2015-05-22T09:04:56.047528Z
Changed paths:
M
Merge 193326 "Revert of Add helper to validate JPEG decode subsa..."
(cherry picked from commit 8a5bed0d225efcb2e7fbd96deb2b0307bff04679)
Merge blink r193326 into M43 branch 2357
TBR=vangelis@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=486508
Review URL:
-----------------------------------------------------------------
ti...@google.com
Updating release labels based on merge.
no...@chromium.org
The fix for this issue was #105 and rolled into Chrome on M44, so that's the correct label.
We were crossing release streams here. One change made during this fix got included in M43. I reverted it from M44, but needed to revert it from M43 too, hence #118.
We were crossing release streams here. One change made during this fix got included in M43. I reverted it from M44, but needed to revert it from M43 too, hence #118.
ti...@google.com
Thanks for the breakdown. Updating the labels... once more.
no...@chromium.org
We should move on to marking verify fixed. lcamtuf@ so many test cases. Please verify they're all working to your satisfaction in Chrome M44. I believe a test case reported at mozilla is not fixed.
no...@chromium.org
Think lcamtuf is on leave. Went though all the test cases I could find. All fixed, except for one test case
http://lcamtuf.coredump.cx/ffjpeg
which has multiple renderings, but not for the reasons on this bug and nor is it a security issue, jsut a rendering issue.
Will file a separate bug about that.
which has multiple renderings, but not for the reasons on this bug and nor is it a security issue, jsut a rendering issue.
Will file a separate bug about that.
no...@chromium.org
[Deleted User] <[Deleted User]>
[Empty comment from Monorail migration]
cl...@chromium.org
Bulk update: removing view restriction from closed bugs.
sh...@chromium.org
This bug has been closed for more than 14 weeks. Removing security view restrictions.
For more details visithttps://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
For more details visit
sh...@chromium.org
This bug has been closed for more than 14 weeks. Removing security view restrictions.
For more details visithttps://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
For more details visit
mb...@chromium.org
[Empty comment from Monorail migration]
is...@google.com
This issue was migrated from crbug.com/chromium/398235?no_tracker_redirect=1
[Auto-CCs applied]
[Multiple monorail components: Blink>Image, Internals>Skia]
[Monorail components added to Component Tags custom field.]
[Auto-CCs applied]
[Multiple monorail components: Blink>Image, Internals>Skia]
[Monorail components added to Component Tags custom field.]
Description
The offending image is:
The tool simply renders it on a canvas several hundred times and compares toDataURL().
It sounds awfully like uninitialized memory, but at this point, I don't know what causes this, where the few varying bits are coming from, and had very limited time to investigate / minimize. Filing in case anyone has any ideas or time to kill. Otherwise, I'll follow up when I can.