1823614 - limit the number of scans we allow in jpeg images to some finite value

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Comment 1

[Timothy Nikkel (:tnikkel)]

Bug 1252200 and bug 1252196 talk about this problem more. However using the programs there to generate a testcase jpeg it did not seem to have a large number of scans, either that or I wasn't patient enough to wait for the first few scans to complete, so I'm filing this separately.

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1823614. Limit the number of scans we allow in jpeg images to some finite value. r?aosmond,#gfx-reviewers

We want to provide some finite limit to prevent small jpeg files from being able to tie up cpus for a much larger time than their small size would normally allow. We want to choose a number high enough so that no sane jpeg file would approach it, unless it had been crafted to take advantage of this problem.

Skia's jpeg decoder limit's it to 100:

https://searchfox.org/mozilla-central/rev/f078cd02746b29652c134b144f0629d47e378166/gfx/skia/skia/src/codec/SkJpegDecoderMgr.cpp#33

The OSS_Fuzz targets for libjpeg-turbo limit it to 500:

https://bugzilla.mozilla.org/show_bug.cgi?id=1252196#c11

Depends on D173119

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e5254e7728e
Limit the number of scans we allow in jpeg images to some finite value. r=gfx-reviewers,bradwerth

Comment 4

[Iulian Moraru]

Backed out for causing reftest failures on pattern-transformed.

• Push with failures

• Failure log

• Backout link

Comment 5

[Timothy Nikkel (:tnikkel)]

I'm not sure this could cause those failures. There is no jpegs in those tests.

Comment 6

[Iulian Moraru]

There is something strange going on. I first backed out Bug 1824025 but I did the backout before seeing this backfills that is why I re-landed bug 1824025 and backed out this bug. Most likely I will re-land your patch.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12d26e785e81
Limit the number of scans we allow in jpeg images to some finite value. r=gfx-reviewers,bradwerth a=reland

Comment 8

[Iulian Moraru]

Sorry for any inconvenience caused by this. Your patch was re-landed.

Comment 9

[Timothy Nikkel (:tnikkel)]

(In reply to Iulian Moraru from comment #6)

There is something strange going on. I first backed out Bug 1824025 but I did the backout before seeing this backfills that is why I re-landed bug 1824025 and backed out this bug. Most likely I will re-land your patch.

Ah, I see. In the backfills link if I open the full log of an Rs2 run on the push for bug 1824025 then I do not see the pattern-transformed* tests as being run, so they must have moved chunks unexpectedly?

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/12d26e785e81

Comment 11

[Timothy Nikkel (:tnikkel)]

Stumbled upon the max scans that imagemagick allows so figured I'd record it here for posterity: 1024

https://github.com/ImageMagick/ImageMagick/blob/f7b5682435d37ad5ea8142d69629c93228e6376d/coders/jpeg.c#L112