Closed Bug 1223002 Opened 4 years ago Closed 4 years ago

Graphite 2 instruction parameter validation bypass


(Core :: Graphics: Text, defect)

Not set



Tracking Status
firefox42 --- unaffected
firefox43 + verified
firefox44 + verified
firefox45 + verified
firefox-esr38 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- unaffected
b2g-v2.5 --- fixed
b2g-v2.2r --- unaffected
b2g-master --- fixed


(Reporter: hofusec, Assigned: jfkthame)



(Keywords: regression, sec-critical)


(5 files)

Attached file
With special CNTXT_ITEM instructions its possible to circumvent the validation of parameter for internal instructions in the Graphite smart font system.
This gives a huge attack surface which can exploited for example to a reliable jump to an arbitrary address with a special crafted font.
I'm sure its also possible to bypass ASLR with this bug.

With the current nightly 64bit and windows 7 the poc jumps to 0x4141414141414141.
Group: core-security → gfx-core-security
Keywords: sec-critical
Attached file call_stack.txt
Ever confirmed: true
Martin, here's one that sounds like it wants fixing ASAP.
Flags: needinfo?(martin_hosken)
This is fixed upstream, so we just need to take the most recent fixes from there.
Attachment #8688500 - Flags: review?(jdaggett)
Assignee: nobody → jfkthame
Running the testcase here revealed a bug on our side when gr_face initialization fails; we end up asserting on shutdown. So let's fix that while we're here.
Attachment #8688503 - Flags: review?(jdaggett)
Given the fixes that have gone into 1.3.4 and this following bug, I would recommend applying these to gecko 44 and even 43. But that's you guys' call :)
Flags: needinfo?(martin_hosken)
Comment on attachment 8688503 [details] [diff] [review]
Always call ReleaseGrFace to balance GetGrFace, even if the face is null

Review of attachment 8688503 [details] [diff] [review]:

Hmmm, calling release on a null face seems sort of weird. So, r+ with a comment explaining the necessity of always calling release.
Attachment #8688503 - Flags: review?(jdaggett) → review+
Attachment #8688500 - Flags: review?(jdaggett) → review+
I've confirmed that the testcase attached here crashes current FF43 (Beta) and 44 (Developer Edition) releases.[1] It doesn't appear to hurt FF42 (Release) or 38esr, though that doesn't necessarily mean they are immune... it could be that older graphite2 releases would be vulnerable to a slight variation of the example.

[1] E.g.
Comment on attachment 8688500 [details] [diff] [review]
Cherry-pick post-1.3.4 bugfixes for graphite2 from upstream

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The patch doesn't directly show how to construct an exploit, but does hint at a specific part of the font tables that might be worth exploring.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No. (We could land the testcase as a crashtest later.)

Which older supported branches are affected by this flaw?
FF43 and FF44.

If not all supported branches, which bug introduced the flaw?
Bug 1200098 (major update of graphite2 library).

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Aurora and Beta are currently on graphite2-1.3.3. To backport, we should take a combined patch with bug 1220591 (update to 1.3.4) plus this bug (post-1.3.4 fixes). I'll post a combined patch.

How likely is this patch to cause regressions; how much testing does it need?
Minimal risk of regressions; this just adds error-checking to the graphite2 code so as to reject bad font tables before anything bad happens.
Attachment #8688500 - Flags: sec-approval?
This is the combined backport of bug 1220591 (update to 1.3.4) and this bug, for aurora and beta branches.
Comment on attachment 8689196 [details] [diff] [review]
Update graphite2 to release 1.3.4 plus post-release bugfixes from upstream

Approval Request Comment

(See sec-approval comment above.)
Attachment #8689196 - Flags: approval-mozilla-beta?
Attachment #8689196 - Flags: approval-mozilla-aurora?
Comment on attachment 8688500 [details] [diff] [review]
Cherry-pick post-1.3.4 bugfixes for graphite2 from upstream

sec-approval+ for trunk and other approvals given.
Attachment #8688500 - Flags: sec-approval? → sec-approval+
Attachment #8689196 - Flags: approval-mozilla-beta?
Attachment #8689196 - Flags: approval-mozilla-beta+
Attachment #8689196 - Flags: approval-mozilla-aurora?
Attachment #8689196 - Flags: approval-mozilla-aurora+
Bug 1223002 - Cherry-pick post-1.3.4 bugfixes for graphite2 from upstream. r=jdaggett
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Oops, I failed to push the second patch here when landing to inbound. (This part isn't a security issue, it's just so we don't mistakenly think we've leaked a face that we actually failed to create.) Coming up shortly...
Bug 1223002 - Always call ReleaseGrFace to balance GetGrFace, even if the face is null. r=jdaggett
Group: gfx-core-security → core-security-release
Reproduced the initial crash using old Nightly x64 (2015-11-08) on Ubuntu 14.04 64bit ( console output) and Windows 7 64-bit (bp-08f1ac46-64d5-4658-94bd-fbc582151207), verified that the crash does not reproduce anymore using latest Nightly 45.0a1, latest Developer Edition 44.0a2 and Firefox 43 beta 9 across platforms (Windows 7 64-bit, Ubuntu 14.04 64-bit and Mac OS X 10.11).
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.