752662 - Graphite 2 use-after-free crash

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: testcase

The testcase is reproducible with a ASAN enabled build. If necessary, reload the testcase a few times. 

If you have not a ASAN enabled build yet, follow the instructions described here: https://developer.mozilla.org/en/Building_Firefox_with_Address_Sanitizer

Comment 1

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 2

[martin_hosken]

Fixed in repo. Keep 'em coming :)

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: patch, cherry-pick fix from graphite repository

Comment 4

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8789a1dc555d

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8789a1dc555d

Comment 6

[Alex Keybl [:akeybl]]

Is the ESR10 branch affected by this sg:high?

Comment 7

[Jonathan Kew [:jfkthame]]

No; the graphite lib isn't present there.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Verified fixed in my May 10 post-fix ASAN build on OS X.

Comment 9

[Daniel Veditz [:dveditz]]

If we have graphite2 in Firefox 13 we need to uplift this patch.

Comment 10

[Jonathan Kew [:jfkthame]]

Comment on attachment 621912 [details] [diff] [review]
patch, cherry-pick fix from graphite repository

[Approval Request Comment]
Bug caused by (feature/regressing bug #): graphite font-shaping library

User impact if declined: users who enable graphite may be vulnerable to a crash caused by a malicious/buggy font

Testing completed (on m-c, etc.): see comment #8

Risk to taking this patch (and alternatives if risky): minimal risk, just adds a missing bounds check

String or UUID changes made by this patch: none

Nominating this for aurora (14) on the basis of comment #9.

The patch here doesn't apply to beta (13), because we took a complete graphite refresh for mozilla-14 in bug 746975.

IMO, the security concern here is relatively minor, given that graphite is preffed off by default and only likely to be enabled by a small minority of users with a specific interest in trying it (primarily for "exotic" writing systems). I see no point in trying to backport just this specific patch to the mozilla-13 version; if we're sufficiently concerned about it, we should backport the complete graphite update (but personally I don't think it is important to do so for a preffed-off feature).

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 621912 [details] [diff] [review]
patch, cherry-pick fix from graphite repository

[Triage Comment]
Approving for Aurora 14 given the sg:high rating.

Comment 12

[Jonathan Kew [:jfkthame]]

Transplanted to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/9e96c2d39ced