1789729 - Creating extremely large 3D textures in WebGL could cause buffer overrun in flawed OpenGL drivers (not ANGLE)

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, task)

Description

[Ashley Hale [:ahale]]

Per discussion in bug #1730637 there may be flawed OpenGL drivers in the wild that could suffer a buffer overrun on extremely large 3D textures (exceeding 4GiB or 2GiB depending on how the driver code works). It's also possible that 2D textures >= 32768x32768 could trigger the same behavior, but such flawed drivers are likely to report lower limits for GL_MAX_TEXTURE_SIZE, so this is mostly a 3D texture issue.

We should implement the gfx.webgl.max-per-texture-size-mib pref discussed there, to check a rejection limit before reserving texture storage or uploading an image. The safest default value would likely be 256 to account for possible format conversions (e.g. GL_RED8->GL_RGBA8 being *4) and possible use of mipmaps (which is approximately *1.999999 on a 1D texture or *1.333333 on the size of a 2D texture or *1.142857 on a 3D texture) while remaining <= INT_MAX (2GiB - 1). There should be a detailed error message when a texture is rejected, making mention of the pref, so that any web developers who genuinely need WebGL to operate on extremely large textures can do so for their own personal use.

If use of extremely large textures in WebGL turns out to be widespread, we may need a different solution (e.g. pref based on allowlist of drivers known to do the allocation math correctly), but in terms of safe-by-default it is likely best to limit texture size on all platforms until we make such a decision.

This bug may be relevant on all platforms, but Angle on Windows D3D11 is likely unaffected (testing still worthwhile).

Comment 1

[Ashley Hale [:ahale]]

A pref default of 1024 is probably safe enough - this would allow drivers to do certain conversions (e.g. GL_RGB8 to GL_RGBA8 is *1.3333333, but not things like GL_RED8 to GL_RGBA8 which is *4) and generate mipmaps (which is approximately a *1.3333333 multiplier, and if combined with GL_RGB8 conversion means *1.7777778) while staying under INT_MAX. It's unlikely that drivers are doing GL_RED8 to GL_RGBA8 conversion or GL_RG8 to GL_RGBA8 conversion, as only GL_RGB has a history of not being natively supported.

Comment 2

[Ashley Hale [:ahale]]

Related bug #1239446

Comment 3

[Ashley Hale [:ahale]]

I have draft code to implement this, producing this error in the devtools Console on the PoC from #1730637:
WebGL warning: texStorage(Multisample)?: Texture size too large; base image mebibytes > webgl.max-size-per-texture-mib

I've also tested it on WebGL Aquarium ( https://webglsamples.org/aquarium/aquarium.html ) with different values for webgl.max-size-per-texture-mib and it behaves as expected (tested values: 1024 behaves normally, 1 prevents the background scene texture from loading, 0 prevents all textures from loading, 1000000000 and -1000000000 both behave normally).

Will prepare a Diff shortly.

Comment 4

[Ashley Hale [:ahale]]

Attachment: Bug 1789729 - Implement webgl.max-size-per-texture-mib r?jgilbert

Comment 5

[Ashley Hale [:ahale]]

Comment on attachment 9293822 [details]
Bug 1789729 - Implement webgl.max-size-per-texture-mib r?jgilbert

Security Approval Request

• How easily could an exploit be constructed based on the patch?: This patch is in response to a possible exploit vector discussed in bug 1730637 which was made publicly visible (to my surprise) after it was fixed, so the bulls-eye is already painted. It's not entirely clear if an exploit based on this patch would actually work against any OpenGL drivers, this is aimed at defense-in-depth as OpenGL driver vendors are not as concerned with security as we are. If an exploit is viable it would likely be specific to certain GPU vendors and GPU models (which means Mesa on Linux and Qualcomm GPU drivers on Android would be the only interesting targets for an attacker, and it's not clear if either of those are vulnerable).
• 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?: all?
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Backports should be easy.
• How likely is this patch to cause regressions; how much testing does it need?: The chosen default value of the new pref (1GiB before mipmaps and format conversions) shouldn't be able to break any web content we know of. It's possible this patch could break web pages that visualize extremely large voxel data sets such as medical imaging, but it's not clear if any usage of that nature exists (and there are reasons such software would want to use smaller textures than this, so even if such software exists it's likely not bumping into this limit).
• Is Android affected?: Unknown

Comment 6

[Tom Ritter [:tjr]]

Comment on attachment 9293822 [details]
Bug 1789729 - Implement webgl.max-size-per-texture-mib r?jgilbert

Approved to land and request uplift.

Comment 7

[Ashley Hale [:ahale]]

Comment on attachment 9293822 [details]
Bug 1789729 - Implement webgl.max-size-per-texture-mib r?jgilbert

Beta/Release Uplift Approval Request

• User impact if declined: Potential security issue, this is defense in depth to protect users against flawed drivers.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• 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): Rare issue, small patch for a potential issue.
• String changes made/needed:
• Is Android affected?: Yes

Comment 8

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

Implement webgl.max-size-per-texture-mib r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/1e106796d80a09440185e4d5bcdd431e28dea125
https://hg.mozilla.org/mozilla-central/rev/1e106796d80a

Comment 9

[Pascal Chevrel:pascalc]

Comment on attachment 9293822 [details]
Bug 1789729 - Implement webgl.max-size-per-texture-mib r?jgilbert

Approved for 106.0b8, thanks.

Comment 10

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/373be34fc255

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9293822 [details]
Bug 1789729 - Implement webgl.max-size-per-texture-mib r?jgilbert

Approved for 102.4esr.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/1f5b5639147d