Bug 739925 (CVE-2011-3062)

OTS off-by-one may result in arbitrary code execution

RESOLVED FIXED in Firefox 12

Status

()

Firefox
General
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Huzaifa Sidhpurwala, Assigned: jfkthame)

Tracking

11 Branch
Firefox 12
All
Linux
Points:
---

Firefox Tracking Flags

(firefox11 affected, firefox12+ fixed, firefox13+ fixed, firefox14+ fixed, firefox-esr1012+ fixed)

Details

(Whiteboard: [sg:critical][qa-])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
This is chromium bug:
https://code.google.com/p/chromium/issues/detail?id=116524
which states that  jruderman and dveditz are cced, but since i did not find an open bug, i decided to open one. (Apologies if this is a dupe.)

Patch available at:
http://code.google.com/p/ots/source/detail?r=83#

snippet from upstream bug:
There's an off-by-one vulnerability in OpenType Sanitiser (http://code.google.com/p/ots/). The bug was found using AddressSanitizer.

See the following code from src/gpos.cc:

--- cut ---
30:   GPOS_TYPE_RESERVED = 10

[...]

60: const ots::LookupSubtableParser::TypeParser kGposTypeParsers[] = {
61:   {GPOS_TYPE_SINGLE_ADJUSTMENT, ParseSingleAdjustment},
62:   {GPOS_TYPE_PAIR_ADJUSTMENT, ParsePairAdjustment},
63:   {GPOS_TYPE_CURSIVE_ATTACHMENT, ParseCursiveAttachment},
64:   {GPOS_TYPE_MARK_TO_BASE_ATTACHMENT, ParseMarkToBaseAttachment},
65:   {GPOS_TYPE_MARK_TO_LIGATURE_ATTACHMENT, ParseMarkToLigatureAttachment},
66:   {GPOS_TYPE_MARK_TO_MARK_ATTACHMENT, ParseMarkToMarkAttachment},
67:   {GPOS_TYPE_CONTEXT_POSITIONING, ParseContextPositioning},
68:   {GPOS_TYPE_CHAINED_CONTEXT_POSITIONING, ParseChainedContextPositioning},
69:   {GPOS_TYPE_EXTENSION_POSITIONING, ParseExtensionPositioning}
70: };
71: 
72: const ots::LookupSubtableParser kGposLookupSubtableParser = {
73:   GPOS_TYPE_RESERVED, GPOS_TYPE_EXTENSION_POSITIONING, kGposTypeParsers
74: };
--- cut ---

The kGposTypeParsers table has 9 items, however a value of 10 (GPOS_TYPE_RESERVED) is used as the num_types value of the kGposLookupSubtableParser structure. This can later lead to an out-of-bounds read and execution of an uninitialized function pointer from kGposTypeParsers[9].parser (see src/layout.cc):

1153: bool LookupSubtableParser::Parse(const OpenTypeFile *file, const uint8_t *data,
1154:                                  const size_t length,
1155:                                  const uint16_t lookup_type) const {
1156:   for (unsigned i = 0; i < num_types; ++i) {
1157:     if (parsers[i].type == lookup_type && parsers[i].parse) {
1158:       if (!parsers[i].parse(file, data, length)) {
1159:         return OTS_FAILURE();
1160:       }
1161:       return true;
1162:     }
1163:   }
1164:   return OTS_FAILURE();
1165: }

The vulnerable behavior can be potentially used to get a direct control over the application's execution flow.
(Reporter)

Updated

5 years ago
Severity: normal → critical
Hardware: x86_64 → All
critical assuming we use the part of OTS affected by this bug (we do have the code in our tree).

The patch is small and safe, we should try to get it into Firefox 12 since Chrome has already shipped this fix.

We should also plan a general update to a newer OTS to pick up any other unidentified security fixes. That should go in a separate bug and not be slammed into a beta.
Assignee: nobody → jfkthame
Status: UNCONFIRMED → NEW
status-firefox-esr10: --- → affected
status-firefox11: --- → affected
status-firefox12: --- → affected
status-firefox13: --- → affected
status-firefox14: --- → affected
tracking-firefox-esr10: --- → 12+
tracking-firefox12: --- → +
tracking-firefox13: --- → +
tracking-firefox14: --- → +
Ever confirmed: true
Whiteboard: [sg:critical]
(Assignee)

Comment 2

5 years ago
Created attachment 610202 [details] [diff] [review]
patch, port the sanitizer bug-fixes from upstream rev.83
Attachment #610202 - Flags: review?(jdaggett)
(Assignee)

Comment 3

5 years ago
(In reply to Daniel Veditz [:dveditz] from comment #1)
> We should also plan a general update to a newer OTS to pick up any other
> unidentified security fixes. That should go in a separate bug and not be
> slammed into a beta.

We do take OTS updates fairly promptly, most recently to rev.81 in bug 730190.

Updated

5 years ago
Attachment #610202 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/baf59f769aed
Target Milestone: --- → Firefox 14
(Assignee)

Comment 5

5 years ago
Comment on attachment 610202 [details] [diff] [review]
patch, port the sanitizer bug-fixes from upstream rev.83

[Approval Request Comment]
Regression caused by (bug #): n/a - upstream OTS bug

User impact if declined: potentially exploitable security vulnerability

Testing completed (on m-c, etc.): only just landed, but already in upstream OTS/chromium

Risk to taking this patch (and alternatives if risky): minimal risk, just corrects array-size value to avoid out-of-bounds read

String changes made by this patch: none
Attachment #610202 - Flags: approval-mozilla-esr10?
Attachment #610202 - Flags: approval-mozilla-beta?
Attachment #610202 - Flags: approval-mozilla-aurora?
Do we have a testcase to use for verification of the fix anywhere?

Comment 7

5 years ago
There's a testcase in https://code.google.com/p/chromium/issues/detail?id=116524 but I don't know if it works in the browser. Do you have a gmail / google code account that could be CCed on that bug?
I have albill@gmail.com.
(Reporter)

Comment 9

5 years ago
(In reply to Jesse Ruderman from comment #7)
> There's a testcase in
> https://code.google.com/p/chromium/issues/detail?id=116524 but I don't know
> if it works in the browser. Do you have a gmail / google code account that
> could be CCed on that bug?

The test cases wont really work in the browser, i can attach them to this bug if needed though
(Reporter)

Comment 10

5 years ago
This is CVE-2011-3062
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/mozilla-central/rev/baf59f769aed
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Alias: CVE-2011-3062
We should fix this on all branches since it has been checked into Chromium.
Once this has a couple of days to bake, we expect to approve for all branches.
Comment on attachment 610202 [details] [diff] [review]
patch, port the sanitizer bug-fixes from upstream rev.83

[Triage Comment]
We haven't heard of any fallout from this change - approved for all branches.
Attachment #610202 - Flags: approval-mozilla-esr10?
Attachment #610202 - Flags: approval-mozilla-esr10+
Attachment #610202 - Flags: approval-mozilla-beta?
Attachment #610202 - Flags: approval-mozilla-beta+
Attachment #610202 - Flags: approval-mozilla-aurora?
Attachment #610202 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 15

5 years ago
Pushed to branches:
https://hg.mozilla.org/releases/mozilla-aurora/rev/4696c6beca3d
https://hg.mozilla.org/releases/mozilla-beta/rev/63e71ea24257
https://hg.mozilla.org/releases/mozilla-esr10/rev/9c6a6651bc1f
status-firefox-esr10: affected → fixed
status-firefox12: affected → fixed
status-firefox13: affected → fixed
status-firefox14: affected → fixed
Target Milestone: Firefox 14 → Firefox 12
Is there something QA can do to verify this fix?
Whiteboard: [sg:critical] → [sg:critical][qa?]
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #16)
> Is there something QA can do to verify this fix?

Apparently not.
Whiteboard: [sg:critical][qa?] → [sg:critical][qa-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.