Last Comment Bug 739925 - (CVE-2011-3062) OTS off-by-one may result in arbitrary code execution
(CVE-2011-3062)
: OTS off-by-one may result in arbitrary code execution
Status: RESOLVED FIXED
[sg:critical][qa-]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: 11 Branch
: All Linux
: -- critical (vote)
: Firefox 12
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-28 03:12 PDT by Huzaifa Sidhpurwala
Modified: 2012-05-18 13:26 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
fixed
+
fixed
+
fixed
12+
fixed


Attachments
patch, port the sanitizer bug-fixes from upstream rev.83 (2.31 KB, patch)
2012-03-28 10:47 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr10+
Details | Diff | Review

Description Huzaifa Sidhpurwala 2012-03-28 03:12:02 PDT
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.
Comment 1 Daniel Veditz [:dveditz] 2012-03-28 10:22:28 PDT
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.
Comment 2 Jonathan Kew (:jfkthame) 2012-03-28 10:47:55 PDT
Created attachment 610202 [details] [diff] [review]
patch, port the sanitizer bug-fixes from upstream rev.83
Comment 3 Jonathan Kew (:jfkthame) 2012-03-28 10:51:20 PDT
(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.
Comment 5 Jonathan Kew (:jfkthame) 2012-03-28 11:18:42 PDT
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
Comment 6 [On PTO until 6/29] 2012-03-28 16:22:03 PDT
Do we have a testcase to use for verification of the fix anywhere?
Comment 7 Jesse Ruderman 2012-03-28 16:35:20 PDT
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?
Comment 8 [On PTO until 6/29] 2012-03-28 18:09:02 PDT
I have albill@gmail.com.
Comment 9 Huzaifa Sidhpurwala 2012-03-28 20:35:46 PDT
(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
Comment 10 Huzaifa Sidhpurwala 2012-03-28 20:46:35 PDT
This is CVE-2011-3062
Comment 11 Jonathan Kew (:jfkthame) 2012-03-29 09:14:56 PDT
https://hg.mozilla.org/mozilla-central/rev/baf59f769aed
Comment 12 [On PTO until 6/29] 2012-03-29 11:19:25 PDT
We should fix this on all branches since it has been checked into Chromium.
Comment 13 Alex Keybl [:akeybl] 2012-03-29 15:20:11 PDT
Once this has a couple of days to bake, we expect to approve for all branches.
Comment 14 Alex Keybl [:akeybl] 2012-04-02 11:01:11 PDT
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.
Comment 16 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-04-16 09:34:25 PDT
Is there something QA can do to verify this fix?
Comment 17 [On PTO until 6/29] 2012-04-24 13:35:56 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #16)
> Is there something QA can do to verify this fix?

Apparently not.

Note You need to log in before you can comment on or make changes to this bug.