1784835 - use checkedint in webp encoder

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Timothy Nikkel (:tnikkel)]

Input to this function is unsigned int 32, input to webp encoder functions are signed int 32. I also noticed a uint32 * uint32 mult that isn't checked for overflow.

Static analysis from https://phabricator.services.mozilla.com/D153578 bug 1782877 found this.

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1784835. Use checkedint in webp encoder to avoid overflow. r?aosmond

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1511670

Comment 3

[Daniel Veditz [:dveditz]]

We're not sure how much our surrounding code protects us from malicious values getting down this deep, so we'll guess sec-high as worst case.

Comment 4

[Timothy Nikkel (:tnikkel)]

Comment on attachment 9289839 [details]
Bug 1784835. Use checkedint in webp encoder to avoid overflow. r?aosmond

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not hard to trace the patch back to a web content call to canvas.toDataUrl("image/webp") using an image that overflows int in size.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: 96 and newer
• If not all supported branches, which bug introduced the flaw?: Bug 1511670
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: trivial
• How likely is this patch to cause regressions; how much testing does it need?: very safe, only affects cases that overflow 32bit int
• Is Android affected?: Yes

Comment 5

[Tom Ritter [:tjr]]

Comment on attachment 9289839 [details]
Bug 1784835. Use checkedint in webp encoder to avoid overflow. r?aosmond

Approved to land and uplift

Comment 6

[Timothy Nikkel (:tnikkel)]

Comment on attachment 9289839 [details]
Bug 1784835. Use checkedint in webp encoder to avoid overflow. r?aosmond

Beta/Release Uplift Approval Request

• User impact if declined: security bug
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): checks for overflow in 32bit ints, images that large that are not designed to attack are basically non-existent
• String changes made/needed:
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: security bug
• User impact if declined: security bug
• Fix Landed on Version: 106
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): checks for overflow in 32bit ints, images that large that are not designed to attack are basically non-existent

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Use checkedint in webp encoder to avoid overflow. r=aosmond
https://hg.mozilla.org/integration/autoland/rev/10b4cfef2396d97ce9ab9ed0d7654efb7bf98129
https://hg.mozilla.org/mozilla-central/rev/10b4cfef2396

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9289839 [details]
Bug 1784835. Use checkedint in webp encoder to avoid overflow. r?aosmond

Approved for 105.0b3 and 102.3esr.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2276038aab0f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/e773b8e40a33