My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 116524: Security: Off-by-one in OTS resulting in arbitrary code execution
1 person starred this issue and may be notified of changes. Back to list
 
Reported by mjurc...@google.com, Mar 2, 2012
VULNERABILITY DETAILS

There's an off-by-one vulnerability in OpenType Sanitiser (https://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.

Log from ASAN:

=================================================================
==1119== ERROR: AddressSanitizer global-buffer-overflow on address 0x000000754d70 at pc 0x51447a bp 0x7fff67fe8db0 sp 0x7fff67fe8da8
READ of size 2 at 0x000000754d70 thread T0
  #0 0x51447a ots::LookupSubtableParser::Parse()
  #1 0x5161fe (anonymous namespace)::ParseLookupTable()
  #2 0x515ab8 ots::ParseLookupListTable()
  #3 0x4f9f7d ots::ots_gpos_parse()
  #4 0x4b5e6f (anonymous namespace)::ProcessGeneric()
  #5 0x4b5384 (anonymous namespace)::ProcessTTF()
  #6 0x4b4107 ots::Process()
  #7 0x4b344b main
  #8 0x7f2927a2cd5d __libc_start_main
0x000000754d70 is located 0 bytes to the right of global variable '_ZN12_GLOBAL__N_1L16kGposTypeParsersE (ots/src/gpos.cc)' (0x754ce0) of size 144
==1119== ABORTING
Stats: 0M malloced (0M for red zones) by 710 calls
Stats: 0M realloced by 0 calls
Stats: 0M freed by 29 calls
Stats: 0M really freed by 0 calls
Stats: 20M (5121 full pages) mmaped in 5 calls
  mmaps   by size class: 8:16383; 9:8191; 10:4095; 11:2047; 17:32;
  mallocs by size class: 8:696; 9:9; 10:3; 11:1; 17:1;
  frees   by size class: 8:26; 9:2; 10:1;
  rfrees  by size class:
Stats: malloc large: 1 small slow: 5
Shadow byte and word:
  0x1000000ea9ae: f9
  0x1000000ea9a8: 00 00 00 00 00 00 f9 f9
More shadow bytes:
  0x1000000ea988: 00 00 00 00 f9 f9 f9 f9
  0x1000000ea990: 00 00 00 00 00 00 00 f9
  0x1000000ea998: f9 f9 f9 f9 00 00 00 00
  0x1000000ea9a0: 00 00 00 00 00 00 00 00
=>0x1000000ea9a8: 00 00 00 00 00 00 f9 f9
  0x1000000ea9b0: f9 f9 f9 f9 00 00 00 00
  0x1000000ea9b8: 00 00 00 00 00 00 f9 f9
  0x1000000ea9c0: f9 f9 f9 f9 00 00 f9 f9
  0x1000000ea9c8: f9 f9 f9 f9 00 00 f9 f9


VERSION

OpenType Sanitiser r82, any operating system or Chrome version.


REPRODUCTION CASE

See attachments. They reproduce when used with ot-sanitise:

./ot-sanitise >/dev/null 1015sn.otf.asan.5a.2

1015sn.otf.asan.5a.2
44.0 KB   Download
24248.8683.ttf.asan.5a.55
10.2 KB   Download
Kinnari.otf.asan.5a.172
125 KB   Download
Mar 2, 2012
#1 skylined@chromium.org
Upstream tracking bug to get some attention:
https://code.google.com/p/ots/issues/detail?id=3
Mar 2, 2012
#2 skylined@chromium.org
@bashi: it looks like you may be the right guy to look at this bug.
Cc: ba...@chromium.org
Mar 2, 2012
#3 ba...@chromium.org
Thank you for finding the bug. It's obviously my fault. Attached a patch that will fix the bug. Could you confirm it? 
fix.patch
863 bytes   View   Download
Status: Started
Owner: ba...@chromium.org
Cc: a...@chromium.org yusukes@chromium.org
Mar 2, 2012
#4 mjurc...@google.com
Technically it is fine, as long as GPOS_TYPE_RESERVED isn't (erroneously or not) changed to something different from number of the array size + 1 at some point in time. I think it would be safer to directly use the number of cells in the table (i.e. sizeof(kGposTypeParsers)/sizeof(kGposTypeParsers[0]) in this context, especially if there's a macro for that in Chromium.

If nothing else, this patch is correct and fixes the issue.
Mar 3, 2012
#5 ba...@chromium.org
Thanks. GPOS_TYPE_RESERVED is unlikely changed, but I agree with you for using the number of cells in the table. I uploaded the patch for review.
Mar 3, 2012
#6 bugdro...@chromium.org
Commit: be8c6381b93a634d6c67a6e3d52c00df864ed461
 Email: bashi@chromium.org@a4e77c2c-9104-11de-800e-5b313e0d2bf3

[OTS] Fix off-by-one in GSUB/GPOS parser

Use the number of cells in the sub-parser table for number of lookup type.

BUG=chromium:116524
TEST=ran test_{un,}malicious_fonts.sh
Review URL: http://codereview.chromium.org/9581044

git-svn-id: http://ots.googlecode.com/svn/trunk@83 a4e77c2c-9104-11de-800e-5b313e0d2bf3

M	src/gpos.cc
M	src/gsub.cc
Mar 3, 2012
#7 skylined@chromium.org
Assigning SecSeverity-High, assuming OTS is in the renderer? Please let me know if it runs in the browser.
Labels: SecSeverity-High
Mar 3, 2012
#8 yusukes@chromium.org
@skylined It runs in the renderer.

The fix has been landed in ots repo https://code.google.com/p/ots/source/list (r83). I'll update Chrome DEPS once the tree is opened (since bashi@ does not have corp network access right now).

Mar 3, 2012
#9 ba...@chromium.org
OTS is in the renderer. I couldn't roll the fix to Chromium now. yusukes@ is going to roll the fix.
Mar 3, 2012
#10 ba...@chromium.org
@yusukes -- thank you so much for your help!
Mar 3, 2012
#11 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=124860

------------------------------------------------------------------------
r124860 | yusukes@google.com | Sat Mar 03 04:56:40 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=124860&r2=124859&pathrev=124860

Update ots to r83.

BUG=116524
TEST=none
TBR=bashi@chromium.org

Review URL: https://chromiumcodereview.appspot.com/9584048
------------------------------------------------------------------------
Mar 3, 2012
#12 yusukes@chromium.org
(No comment was entered for this change.)
Status: Fixed
Mar 3, 2012
#13 infe...@chromium.org
Is this a recent regression ? Did this affect m17 stable ?
Status: FixUnreleased
Labels: -Restrict-View-SecurityTeam -Pri-0 -Area-Undefined Restrict-View-SecurityNotify Pri-1 Area-Internals Merge-Approved OS-All
Mar 3, 2012
#14 ba...@chromium.org
This affects m17 stable. Can I merge the change to m17/m18?
Mar 3, 2012
#15 scarybea...@gmail.com
Too late for M17 but please do go ahead and merge to M18.
Labels: Mstone-18
Mar 3, 2012
#16 ba...@chromium.org
I see. It's difficult to me to merge the change right now because I don't have VPN access and I don't remember my SVN password.. Is it ok to merge on Monday? Or could I ask someone to merge instead?
Mar 3, 2012
#17 scarybea...@gmail.com
Monday is fine. Anything merged by 7PM PST Tuesday will make it into the Wednesday Beta.
Mar 4, 2012
#18 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=124913

------------------------------------------------------------------------
r124913 | bashi@chromium.org | Sun Mar 04 16:32:29 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/DEPS?r1=124913&r2=124912&pathrev=124913

Merge 124860 - Update ots to r83.

BUG=116524
TEST=none
TBR=bashi@chromium.org

Review URL: https://chromiumcodereview.appspot.com/9584048

TBR=yusukes@google.com
Review URL: https://chromiumcodereview.appspot.com/9578009
------------------------------------------------------------------------
Labels: merge-merged-1025
Mar 6, 2012
#19 scarybea...@gmail.com
(No comment was entered for this change.)
Labels: -Merge-Approved Merge-Merged SecImpacts-Stable SecImpacts-Beta
Mar 6, 2012
#20 scarybea...@gmail.com
Firefox uses OTS. Courtesy cc: to jruderman, dveditz.

Guys, we'll have the fix to stable in about two weeks. You can go sooner if you like but don't drop details too obviously.
Cc: jruder...@gmail.com dved...@gmail.com
Mar 21, 2012
#21 scarybea...@gmail.com
ots revved for M18 on src-branch DEPS, r23125

I don't think the change in comment #c18 updated DEPS in the right place.
Mar 24, 2012
#22 scarybea...@gmail.com
(No comment was entered for this change.)
Labels: CVE-2011-3062
Mar 28, 2012
#23 scarybea...@gmail.com
(No comment was entered for this change.)
Cc: alb...@gmail.com
May 15, 2012
#24 cdn@chromium.org
Marking old security bugs Fixed..
Status: Fixed
Oct 13, 2012
#25 bugdro...@chromium.org
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Labels: Restrict-AddIssueComment-Commit
Nov 14, 2012
#26 bugdro...@chromium.org
The following revision refers to this bug:
    http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=23125

------------------------------------------------------------------------
r23125 | cevans@google.com | 2012-03-21T20:48:54.210408Z

------------------------------------------------------------------------
Mar 9, 2013
#27 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Type-Security -Area-Internals -SecSeverity-High -Mstone-18 -SecImpacts-Stable -SecImpacts-Beta Security-Impact-Stable Security-Impact-Beta M-18 Cr-Internals Security-Severity-High Type-Bug-Security
Mar 13, 2013
#28 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: Restrict-View-EditIssue
Mar 13, 2013
#29 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Mar 21, 2013
#30 scarybea...@gmail.com
(No comment was entered for this change.)
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Mar 21, 2013
#31 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Severity-High Security_Severity-High
Mar 21, 2013
#32 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Impact-Stable Security_Impact-Stable
Mar 21, 2013
#33 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Impact-Beta Security_Impact-Beta
Sign in to add a comment

Powered by Google Project Hosting