heap-use-after-free in S_regmatch #15811
Comments
From @dur-randirCreated by @dur-randirWhile fuzzing perl v5.25.8-216-gfbceb79751 built with afl and run /0|$}(?{s!!!})(0@{s!!{s!})/ to perform an access outside of an allocated memory slot. ASAN diagnostics are: ================================================================= 0x60200000db71 is located 1 bytes inside of 10-byte region previously allocated by thread T0 here: GDB reports the following program state: This is sort-of regression in blead since v5.24.0. Before the 794826f is the first bad commit regexec.c: fix #129903: forbid empty pattern in regex code block PL_curpm provides access to the data needed to implement Unfortunately this collides with its use to implement the This meant that a pattern match like /(?{ s!!! })/ will Perl Info |
From @tonycozOn Tue, 17 Jan 2017 02:10:05 -0800, randir wrote:
A better freed by backtrace: 0x60200000dad1 is located 1 bytes inside of 10-byte region [0x60200000dad0,0x60200000dada)
and previously allocated: previously allocated by thread T0 here: I suspect the (?{s!!!}) is moving $_ out from under the calling regexp matching. I don't think this is a security issue. Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @khwilliamsonOn 01/22/2017 04:29 PM, Tony Cook via RT wrote:
I looked at this enough to realize that there are others more familiar The first failure is at a call to This expands to nextchr = ((locinput < reginfo->strend) ? UCHARAT(locinput) : which looks quite safe. But in adding in DEBUG statements, the locinfo |
From @hvdsOn Tue, 17 Jan 2017 02:10:05 -0800, randir wrote:
That's a red herring really, this alternate version avoiding the empty pattern (with some other expansions to try and make clearer what's going on) acts the same before and after that commit: valgrind ./miniperl -Dr -we '$_ = ""; @x = (1); m{ 0 | (?{ s/^// }) 0 @{ foo() } }x; sub foo { s/^/{s/; "x" }' With the -Dr output we see an additional pair of bad reads in pv_escape(), which help to make clear that it's the reallocation of the PV after the second EVAL leaving stale pointers. Around the CALLRUNOPS at regexec.c:6918 we don't appear to make any effort to allow for the possibility our string may move under us, and I'm not sure what it'd make sense to do halfway through a pattern match in any case. For simple-enough cases I think we could maybe leverage the COW framework for this - use COW to make a cheap mirror of the target string, and continue matching against the unaltered original even if someone changes the sv while we're working. Note that that'd be a change of behaviour, which'd probably have at least some fallout. My gut feel is that it'd be relatively minor, and mostly in japh-like code, but I dunno. I also don't know what we can sanely do in the more complex cases where COW can't be used, and it'd be a shame if the choices we make here take us further away from being able to deliver stream-matching in the future. Hugo |
From @iabynOn Wed, Jan 25, 2017 at 04:17:07AM -0800, Hugo van der Sanden via RT wrote:
It can be reduced further to the following (using $s rather than $_ to $s = "a"; and it shows clearly that it's the PVX buffer of $s being freed within I think some sort of COWing does seem to be the way forward. But in any case, I don't think this is a security issue, as the attacker So I propose we move this to the public queue. -- |
From @iabynOn Mon, Feb 20, 2017 at 05:10:48PM +0000, Dave Mitchell wrote:
which I am now doing. -- |
Migrated from rt.perl.org#130569 (status was 'open')
Searchable as RT130569$
The text was updated successfully, but these errors were encountered: