Wrong condition in else-if statement (mozilla-2.0/intl/uconv/util/uscan.c) let you access outside the bounds [0,29] of array tMap

RESOLVED FIXED in mozilla12

Status

()

Core
Internationalization
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Minh Cuong Tran, Assigned: smontagu)

Tracking

Trunk
mozilla12
All
Other
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622232052

Steps to reproduce:

Reading code.


Actual results:

I found this piece of of code at intl/uconv/util/uscan.c:624

=========
  /* Compute TIndex  */
  if(0xd4 == in[7])  
  {
    TIndex = 0;
  } 
  else if((in[7] < 0xa1) && (in[7] > 0xbe)) {/* illegal trailling consonant */
    return PR_FALSE;
  } 
  else {
    static const PRUint8 tMap[] = {
      /*        A1   A2   A3   A4   A5   A6   A7  */
      1,   2,   3,   4,   5,   6,   7,
        /*   A8   A9   AA   AB   AC   AD   AE   AF  */
        0xff,   8,   9,  10,  11,  12,  13,  14,
        /*   B0   B1   B2   B3   B4   B5   B6   B7  */
        15,  16,  17,0xff,  18,  19,  20,  21,
        /*   B8   B9   BA   BB   BC   BD   BE       */
        22,0xff,  23,  24,  25,  26,  27     
    };
    TIndex = tMap[in[7] - 0xa1];
    if(0xff == (0xff & TIndex))
      return PR_FALSE;
  }
=========

tMap is an array with size [0,29]. The else-if statement is
(in[7] < 0xa1) && (in[7] > 0xbe)
but it should be
(in[7] < 0xa1) || (in[7] > 0xbe)

Otherwise you will always jump in else branch and can access outside of [0,29] of array tMap with the statement:
    TIndex = tMap[in[7] - 0xa1];
since else-if will be always false.


Expected results:

Change code into
(in[7] < 0xa1) || (in[7] > 0xbe)
(Reporter)

Updated

6 years ago
Keywords: conversion
In uCnSAlways8BytesDecomposedHangul
Assignee: nobody → smontagu
Component: General → Internationalization
Keywords: conversion
Product: Firefox → Core
QA Contact: general → i18n
Version: 5 Branch → Trunk
(Reporter)

Comment 2

6 years ago
Same problem uCnSAlways8BytesDecomposedHangul for:
  if((in[3] < 0xa1) && (in[3] > 0xbe)) { /* illegal leading consonant */
Ms2ger, want to patch?
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
I assume these characters end up in the string where they could be inspected? Reading ~250 bytes of stack might get an attacker useful information, like a return address that could be used to defeat ASLR defenses. Probably not any sensitive memory from other pages since that would most likely be in heap. Guessing sg:low as a stepping-stone issue.
Whiteboard: [sg:low]
(Assignee)

Comment 6

6 years ago
Created attachment 545619 [details] [diff] [review]
Patch
Attachment #545619 - Flags: review?(VYV03354)
(Assignee)

Comment 7

6 years ago
Created attachment 545621 [details] [diff] [review]
Tests
Comment on attachment 545619 [details] [diff] [review]
Patch

Sorry for forgetting the review request.
Attachment #545619 - Flags: review?(VYV03354) → review+
Comment on attachment 545621 [details] [diff] [review]
Tests

> + *  * octet6 == 0xa7
0xa4

Otherwise looks fine.
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9596d9d2d2b
https://hg.mozilla.org/integration/mozilla-inbound/rev/71370660e59d
Flags: in-testsuite+
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/f9596d9d2d2b
https://hg.mozilla.org/mozilla-central/rev/71370660e59d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.