705663 - Fix braindead WebGLTexture::HasImageInfoAt

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: fix HasImageInfoAt

I have no idea how I've been able to create 3 separate bugs in 3 lines of code. This was done in a rush for bug 635666.

The first bug is that (level <= mMaxLevelWithCustomImages) didn't account for the case where no image at all has been defined, so the array length is zero.

The second bug is that ImageInfoAt(level, 0) should have been ImageInfoAt(level, face).

The third bug is only theoretical: we weren't really checking for integer overflow. This couldn't really be exploited in practice though, as doing so would have required creating arrays larger than addressable space. But since this function is security-critical (see bug 635666) let's be future-proof.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/bc48009a6bbb

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/dc25872e832d
http://hg.mozilla.org/releases/mozilla-beta/rev/f792b7fa1331

Comment 3

[Daniel Veditz [:dveditz]]

I don't think we need to hide this as a security bug, there's no immediate vulnerability here.

Comment 4

[u279076]

Is this something QA can verify?

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

The only useful thing to do here would be to add a unit test; ping me after holidays if you think this is worth it (the likelihood of a regression around here seems low to me). This function is used by webgl.texImage2D so a unit test would do corresponding texImage2D calls hitting the cases that were improperly handled, as described in comment 0.

Comment 6

[u279076]

I defer to your judgement to whether or not a unit test is useful here. Without it, this fix will likely not be verifiable.