Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

heap-use-after-free in Perl_sv_setpvn (sv.c:5002) #15826

Closed p5pRT closed 7 years ago

p5pRT commented 7 years ago

Migrated from rt.perl.org#130624 (status was 'resolved')

Searchable as RT130624$

p5pRT commented 7 years ago

From @dur-randir

Created by @dur-randir

While fuzzing perl v5.25.8-216-gfbceb79751 built with afl and run under libdislocator\, I found the following program

0+substr($a="\2120000"\, 0\, 1\, "\x{1c0}")

to perform an access outside of an allocated memory slot. ASAN diagnostics are​:

================================================================= ==28921==ERROR​: AddressSanitizer​: heap-use-after-free on address 0x60200000dd70 at pc 0x0000004d3b26 bp 0x7ffe77829ff0 sp 0x7ffe778297a0 READ of size 2 at 0x60200000dd70 thread T0   #0 0x4d3b25 in __asan_memmove (/home/afl/afl-asan/perl+0x4d3b25)   #1 0x9740f7 in Perl_sv_setpvn /home/afl/afl-asan/sv.c​:5002​:5   #2 0xa591f9 in Perl_pp_substr /home/afl/afl-asan/pp.c​:3433​:6   #3 0x848051 in Perl_runops_debug /home/afl/afl-asan/dump.c​:2260​:23   #4 0x5f0465 in S_run_body /home/afl/afl-asan/perl.c​:2528​:2   #5 0x5f0465 in perl_run /home/afl/afl-asan/perl.c​:2451   #6 0x522472 in main /home/afl/afl-asan/perlmain.c​:123​:9   #7 0x7fa819d5f2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)   #8 0x43ad59 in _start (/home/afl/afl-asan/perl+0x43ad59)

0x60200000dd70 is located 0 bytes inside of 10-byte region [0x60200000dd70\,0x60200000dd7a) freed by thread T0 here​:   #0 0x4ea4b0 in __interceptor_cfree.localalias.1 (/home/afl/afl-asan/perl+0x4ea4b0)   #1 0x84d387 in Perl_safesysfree /home/afl/afl-asan/util.c​:388​:2   #2 0x979740 in Perl_sv_utf8_upgrade_flags_grow /home/afl/afl-asan/sv.c​:3605​:3   #3 0xa5716c in Perl_pp_substr /home/afl/afl-asan/pp.c​:3399​:3   #4 0x848051 in Perl_runops_debug /home/afl/afl-asan/dump.c​:2260​:23   #5 0x5f0465 in S_run_body /home/afl/afl-asan/perl.c​:2528​:2   #6 0x5f0465 in perl_run /home/afl/afl-asan/perl.c​:2451   #7 0x522472 in main /home/afl/afl-asan/perlmain.c​:123​:9   #8 0x7fa819d5f2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)   #9 0x43ad59 in _start (/home/afl/afl-asan/perl+0x43ad59)

previously allocated by thread T0 here​:   #0 0x4ea668 in malloc (/home/afl/afl-asan/perl+0x4ea668)   #1 0x84c58b in Perl_safesysmalloc /home/afl/afl-asan/util.c​:153​:21   #2 0x9578ab in Perl_sv_grow /home/afl/afl-asan/sv.c​:1601​:17   #3 0x959c1f in S_sv_uncow /home/afl/afl-asan/sv.c​:5277​:17   #4 0x952749 in Perl_sv_force_normal_flags /home/afl/afl-asan/sv.c​:5317​:2   #5 0x97e0e1 in Perl_sv_pvn_force_flags /home/afl/afl-asan/sv.c​:10034​:9   #6 0xa57018 in Perl_pp_substr /home/afl/afl-asan/pp.c​:3396​:9   #7 0x848051 in Perl_runops_debug /home/afl/afl-asan/dump.c​:2260​:23   #8 0x5f0465 in S_run_body /home/afl/afl-asan/perl.c​:2528​:2   #9 0x5f0465 in perl_run /home/afl/afl-asan/perl.c​:2451   #10 0x522472 in main /home/afl/afl-asan/perlmain.c​:123​:9   #11 0x7fa819d5f2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)   #12 0x43ad59 in _start (/home/afl/afl-asan/perl+0x43ad59)

GDB reports the following program state​:

(gdb) bt #0 __memmove_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/../multiarch/memmove-vec-unaligned-erms.S​:306 #1 0x00007f58684ba7d6 in Perl_sv_setpvn (sv=0x7f5866ce0ec8\, ptr=0x7f58669fbff6 "\212\060\060\060\060"\, len=2) at sv.c​:5002 #2 0x00007f586850e454 in Perl_pp_substr () at pp.c​:3433 #3 0x00007f5868424bde in Perl_runops_debug () at dump.c​:2260 #4 0x00007f586831f156 in S_run_body (oldscope=1) at perl.c​:2528 #5 0x00007f586831e6d4 in perl_run (my_perl=0x7f5868905fff) at perl.c​:2451 #6 0x00007f58682d9d3e in main (argc=2\, argv=0x7fff29387d08\, env=0x7fff29387d20) at perlmain.c​:123 (gdb) f 1 #1 0x00007f58684ba7d6 in Perl_sv_setpvn (sv=0x7f5866ce0ec8\, ptr=0x7f58669fbff6 "\212\060\060\060\060"\, len=2) at sv.c​:5002 5002 Move(ptr\,dptr\,len\,char);

Perl Info ``` Flags: category=core severity=medium Site configuration information for perl 5.25.9: Configured by root at Sat Jan 14 02:25:05 MSK 2017. Summary of my perl5 (revision 5 version 25 subversion 9) configuration: Commit id: cbe2fc5001aa59cdc73e04cc35e097a2ecfbeec0 Platform: osname=linux osvers=3.16.0-4-amd64 archname=x86_64-linux uname='linux dorothy 3.16.0-4-amd64 #1 smp debian 3.16.36-1+deb8u2 (2016-10-19) x86_64 gnulinux ' config_args='-des -Dusedevel -DDEBUGGING -Dcc=afl-clang-fast -Doptimize=-O0 -g -ggdb3' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n bincompat5005=undef Compiler: cc='afl-clang-fast' ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2' optimize='-O0 -g -ggdb3' cppflags='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='4.2.1 Compatible Clang 3.9.1 (tags/RELEASE_391/rc2)' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='afl-clang-fast' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/llvm-3.9/bin/../lib/clang/3.9.1/lib /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.24.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.24' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O0 -g -ggdb3 -L/usr/local/lib -fstack-protector-strong' @INC for perl 5.25.9: lib /usr/local/lib/perl5/site_perl/5.25.9/x86_64-linux /usr/local/lib/perl5/site_perl/5.25.9 /usr/local/lib/perl5/5.25.9/x86_64-linux /usr/local/lib/perl5/5.25.9 Environment for perl 5.25.9: HOME=/home/afl LANG=en_US.UTF-8 LANGUAGE=en_US:en LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.22.1/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games PERLBREW_BASHRC_VERSION=0.78 PERLBREW_HOME=/home/afl/.perlbrew PERLBREW_MANPATH=/home/afl/perlbrew/perls/perl-5.22.1/man PERLBREW_PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.22.1/bin PERLBREW_PERL=perl-5.22.1 PERLBREW_ROOT=/home/afl/perlbrew PERLBREW_VERSION=0.78 PERL_BADLANG (unset) SHELL=/usr/bin/zsh ```
p5pRT commented 7 years ago

From @arc

Sergey Aleynikov \perl5\-security\-report@​perl\.org wrote​:

0+substr($a="\2120000"\, 0\, 1\, "\x{1c0}")

Necessary to trigger the bug​:

- the substr is in non-void context - the rhs is non-empty and has SvUTF8 set - the lhs will grow when upgraded to SvUTF8 (so must be at least 5 bytes under our current heuristics)

I've been using this test case (which isn't exactly a reduction) on the grounds that having the arguments stored in variables eliminates a source of potential issues​:

./miniperl -e '$x = "\xE9zzzz"; $y = "\x{100}"; scalar substr $x\, 0\, 1\, $y;

The cause is that the SvPV of the lhs has already been read into a local variable when it gets upgraded\, so when the buffer is reallocated\, that variable becomes a dangling pointer. The attached patch fixes this.

I suspect this may not be a security issue​: even though it could plausibly be triggered by attacker-supplied data\, it's only a read-after-free rather than write-after-free. Does anyone have a view on that?

-- Aaron Crane ** http​://aaroncrane.co.uk/

p5pRT commented 7 years ago

From @arc

0001-RT-130624-heap-use-after-free-in-4-arg-substr.patch ```diff From 4db6b17252bb4b93b0f0dcb57ad2522d77ff710f Mon Sep 17 00:00:00 2001 From: Aaron Crane Date: Tue, 24 Jan 2017 23:39:40 +0000 Subject: [PATCH] RT#130624: heap-use-after-free in 4-arg substr --- pp.c | 4 +++- t/op/substr.t | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pp.c b/pp.c index 657abf7450..121b6c4f57 100644 --- a/pp.c +++ b/pp.c @@ -3396,8 +3396,10 @@ PP(pp_substr) tmps = SvPV_force_nomg(sv, curlen); if (DO_UTF8(repl_sv) && repl_len) { if (!DO_UTF8(sv)) { + /* Upgrade the dest, and recalculate tmps in case the buffer + * got reallocated; curlen may also have been changed */ sv_utf8_upgrade_nomg(sv); - curlen = SvCUR(sv); + tmps = SvPV_nomg(sv, curlen); } } else if (DO_UTF8(sv)) diff --git a/t/op/substr.t b/t/op/substr.t index 83e7baeddc..fc0ea76346 100644 --- a/t/op/substr.t +++ b/t/op/substr.t @@ -22,7 +22,7 @@ $SIG{__WARN__} = sub { } }; -plan(390); +plan(392); run_tests() unless caller; @@ -847,6 +847,15 @@ is $o::count, 1, 'assigning utf8 overload to substr lvalue calls ovld 1ce'; is ${\substr @a, 0}, scalar @a, '\substr @a'; } +# [perl #130624] - heap-use-after-free, observable under asan +{ + my $x = "\xE9zzzz"; + my $y = "\x{100}"; + my $z = substr $x, 0, 1, $y; + is $z, "\xE9", "RT#130624: heap-use-after-free in 4-arg substr (ret)"; + is $x, "\x{100}zzzz", "RT#130624: heap-use-after-free in 4-arg substr (targ)"; +} + } # sub run_tests - put tests above this line that can run in threads -- 2.11.0 ```
p5pRT commented 7 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 7 years ago

From @hvds

On Tue\, 24 Jan 2017 15​:48​:53 -0800\, arc wrote​:

I suspect this may not be a security issue​: even though it could plausibly be triggered by attacker-supplied data\, it's only a read-after-free rather than write-after-free. Does anyone have a view on that?

I'm not sure​: if the attacker can usefully predict or even control something overwriting the freed buffer\, that'll control what wrong text ends up in the target string. It doesn't sound too plausible to me even as I write it​: it feels like a tenuous link in a fragile chain to a gossamer world of the hypothetical. But if it should turn out to be exactly a one-in-a-million chance\, of course\, all bets are off.

Hugo

p5pRT commented 7 years ago

From @arc

Hugo van der Sanden via RT \perl5\-security\-report@​perl\.org wrote​:

On Tue\, 24 Jan 2017 15​:48​:53 -0800\, arc wrote​:

I suspect this may not be a security issue​: even though it could plausibly be triggered by attacker-supplied data\, it's only a read-after-free rather than write-after-free. Does anyone have a view on that?

I'm not sure​: if the attacker can usefully predict or even control something overwriting the freed buffer\, that'll control what wrong text ends up in the target string. It doesn't sound too plausible to me even as I write it​: it feels like a tenuous link in a fragile chain to a gossamer world of the hypothetical. But if it should turn out to be exactly a one-in-a-million chance\, of course\, all bets are off.

OK. Also\, a further argument in favour of it being a security issue​: I think that further allocations can happen between the free and the read\, because the UTF-8 length cache will be updated on the source sv.

In which case\, does this require a CVE?

-- Aaron Crane ** http​://aaroncrane.co.uk/

p5pRT commented 7 years ago

From @iabyn

On Sat\, Jan 28\, 2017 at 02​:13​:16PM +0000\, Aaron Crane wrote​:

Hugo van der Sanden via RT \perl5\-security\-report@​perl\.org wrote​:

On Tue\, 24 Jan 2017 15​:48​:53 -0800\, arc wrote​:

I suspect this may not be a security issue​: even though it could plausibly be triggered by attacker-supplied data\, it's only a read-after-free rather than write-after-free. Does anyone have a view on that?

I'm not sure​: if the attacker can usefully predict or even control something overwriting the freed buffer\, that'll control what wrong text ends up in the target string. It doesn't sound too plausible to me even as I write it​: it feels like a tenuous link in a fragile chain to a gossamer world of the hypothetical. But if it should turn out to be exactly a one-in-a-million chance\, of course\, all bets are off.

OK. Also\, a further argument in favour of it being a security issue​: I think that further allocations can happen between the free and the read\, because the UTF-8 length cache will be updated on the source sv.

As far as I can see​:

IF the string being modified isn't utf8 but contains chars >= 0x80 AND IF the replacement string is utf8\, AND IF upgrading the string to utf8 doesn't fit within the currently   allocated PVX THEN   IF the string has length magic or is overloaded   THEN   IF whatever code gets executed by that magic does a malloc()   of a similar size to to the old string buffer   THEN   the string may end up with partially garbage contents

  ELSE IF the string buffer size is similar to that of an MG struct   OR IF we have no spare PVMGs in the arena AND the old buffer   size is the same as the arena size   THEN   the string may partially end up with garbage contents

So what we have is under very unlikely sets of circumstances\, the string may end of partially containing garbage. That such a corrupted string allows an exploit is a further unlikelihood.

In which case\, does this require a CVE?

My feeling is no. I think we should just apply your patch to blead soonish.

-- The Enterprise successfully ferries an alien VIP from one place to another without serious incident.   -- Things That Never Happen in "Star Trek" #7

p5pRT commented 7 years ago

From @iabyn

On Mon\, Feb 20\, 2017 at 05​:53​:42PM +0000\, Dave Mitchell wrote​:

On Sat\, Jan 28\, 2017 at 02​:13​:16PM +0000\, Aaron Crane wrote​:

Hugo van der Sanden via RT \perl5\-security\-report@​perl\.org wrote​:

On Tue\, 24 Jan 2017 15​:48​:53 -0800\, arc wrote​:

I suspect this may not be a security issue​: even though it could plausibly be triggered by attacker-supplied data\, it's only a read-after-free rather than write-after-free. Does anyone have a view on that?

I'm not sure​: if the attacker can usefully predict or even control something overwriting the freed buffer\, that'll control what wrong text ends up in the target string. It doesn't sound too plausible to me even as I write it​: it feels like a tenuous link in a fragile chain to a gossamer world of the hypothetical. But if it should turn out to be exactly a one-in-a-million chance\, of course\, all bets are off.

OK. Also\, a further argument in favour of it being a security issue​: I think that further allocations can happen between the free and the read\, because the UTF-8 length cache will be updated on the source sv.

As far as I can see​:

IF the string being modified isn't utf8 but contains chars >= 0x80 AND IF the replacement string is utf8\, AND IF upgrading the string to utf8 doesn't fit within the currently allocated PVX THEN IF the string has length magic or is overloaded THEN IF whatever code gets executed by that magic does a malloc() of a similar size to to the old string buffer THEN the string may end up with partially garbage contents

ELSE    IF the string buffer size is similar to that of an MG struct
     OR IF we have no spare PVMGs in the arena AND the old buffer
           size is the same as the arena size
THEN
    the string may partially end up with garbage contents

So what we have is under very unlikely sets of circumstances\, the string may end of partially containing garbage. That such a corrupted string allows an exploit is a further unlikelihood.

In which case\, does this require a CVE?

My feeling is no. I think we should just apply your patch to blead soonish.

Which I've just pushed as v5.25.10-43-g41b1e85\, and am now closing the ticket.

-- More than any other time in history\, mankind faces a crossroads. One path leads to despair and utter hopelessness. The other\, to total extinction. Let us pray we have the wisdom to choose correctly.   -- Woody Allen

p5pRT commented 7 years ago

@iabyn - Status changed from 'open' to 'pending release'

p5pRT commented 7 years ago

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0\, this and 210 other issues have been resolved.

Perl 5.26.0 may be downloaded via​: https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists\, feel free to reopen this ticket.

p5pRT commented 7 years ago

@khwilliamson - Status changed from 'pending release' to 'resolved'