|
|
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
,
Nov 19 2014
Mark, if it's the same issue, should we just close this issue as Invalid?
,
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?
,
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?
,
Jan 22 2015
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).
,
Jun 26 2015
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
,
Jun 26 2015
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 | |||||