Fix braindead WebGLTexture::HasImageInfoAt

RESOLVED FIXED in Firefox 9

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

({regression})

unspecified
mozilla11
regression
Points:
---

Firefox Tracking Flags

(firefox8 wontfix, firefox9 fixed, firefox10 fixed, firefox11 fixed, status1.9.2 unaffected)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

Created attachment 577251 [details] [diff] [review]
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.
Attachment #577251 - Flags: review?(jmuizelaar)
Attachment #577251 - Flags: review?(jmuizelaar) → review+
Attachment #577251 - Attachment is patch: true
(Assignee)

Comment 1

6 years ago
http://hg.mozilla.org/mozilla-central/rev/bc48009a6bbb
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Attachment #577251 - Flags: approval-mozilla-beta?
Attachment #577251 - Flags: approval-mozilla-aurora?

Updated

6 years ago
Attachment #577251 - Flags: approval-mozilla-beta?
Attachment #577251 - Flags: approval-mozilla-beta+
Attachment #577251 - Flags: approval-mozilla-aurora?
Attachment #577251 - Flags: approval-mozilla-aurora+

Updated

6 years ago
Assignee: nobody → bjacob
(Assignee)

Comment 2

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

Updated

6 years ago
status-firefox10: --- → fixed
status-firefox9: --- → fixed
Target Milestone: --- → mozilla11
I don't think we need to hide this as a security bug, there's no immediate vulnerability here.
Blocks: 635666
Group: core-security
status1.9.2: --- → unaffected
status-firefox11: --- → fixed
status-firefox8: --- → wontfix
Keywords: regression
Is this something QA can verify?
Whiteboard: [qa?]
(Assignee)

Comment 5

6 years ago
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.
I defer to your judgement to whether or not a unit test is useful here. Without it, this fix will likely not be verifiable.
Whiteboard: [qa?] → [qa-]
You need to log in before you can comment on or make changes to this bug.