7-byte file causes Perl 5.21.8 to segfault #14401
Comments
From @geeknikI'm still fuzzing a Perl binary that I built from git source on 01/02/2015 CC=/path/to/afl-gcc ./Configure This is perl 5, version 21, subversion 8 (v5.21.8 (v5.21.7-209-g4e27940)) Besides the above information, this version of Perl was compiled using all While fuzzing the perl binary, I found another testcase which causes a geeknik@deb7fuzz:~/findings/perl/fuzzer01/crashes$ /home/geeknik/perl5/perl geeknik@deb7fuzz:~/findings/perl/fuzzer01/crashes$ gdb This is free software: you are free to change and redistribute it. warning: Can't read pathname for load map: Input/output error. hexdump id\:000001\,sig\:11\,src\:009058\,op\:flip1\,pos\:37.gz |
From @geeknikI'm still fuzzing a Perl binary that I built from git source on 01/02/2015 CC=/path/to/afl-gcc ./Configure This is perl 5, version 21, subversion 8 (v5.21.8 (v5.21.7-209-g4e27940)) Besides the above information, this version of Perl was compiled using all While fuzzing the perl binary, I found a 7-byte test case which causes a geeknik@deb7fuzz:~/findings/perl/fuzzer04/crashes$ /home/geeknik/perl5/perl geeknik@deb7fuzz:~/findings/perl/fuzzer04/crashes$ valgrind -q geeknik@deb7fuzz:~/findings/perl/fuzzer04/crashes$ gdb This is free software: you are free to change and redistribute it. warning: Can't read pathname for load map: Input/output error. hexdump output of the test case file: similar test cases that also cause a segfault: 0000000 3333 7e78 3b33 |
From @geeknik |
From @cpansproutOn Mon Jan 05 20:24:11 2015, brian.carpenter@gmail.com wrote:
Nice bug! $ perl5.12 -e '33x -- Father Chrysostomos |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Mon Jan 05 20:24:11 2015, brian.carpenter@gmail.com wrote:
This doesn't need afl-gcc to reproduce: $ ./perl -e '33x~3' This is caused by pp_repeat passing ((size_t)~0) to SvGROW() and then Perl_sv_grow() bumps that up by 1 (per cbcb2a1) to allocate a byte for a COW count. Unfortunately ((size_t)~0)+1 is 0, so Perl_sv_grow() does nothing, and the string is repeated into the original buffer. The following prevents the segfault and produces the expected panic instead: Inline Patchdiff --git a/sv.c b/sv.c
index fe092c4..1a28368 100644
--- a/sv.c
+++ b/sv.c
@@ -1596,7 +1596,8 @@ Perl_sv_grow(pTHX_ SV *const sv, STRLEN newlen)
* caller wanted a nice 2^N sized block and will be annoyed at getting
* 2^N+1 */
if (newlen & 0xff)
- newlen++;
+ if (!++newlen)
+ --newlen;
#endif
#if defined(PERL_USE_MALLOC_SIZE) && defined(Perl_safesysmalloc_size)
Tony |
From @cpansproutOn Mon Jan 05 21:58:35 2015, tonyc wrote:
Is a panic really expected? 5.12 just died with ‘Out of memory!’ -- Father Chrysostomos |
From @ppisarOn 2015-01-06, Tony Cook via RT <perlbug-followup@perl.org> wrote:
newlen's type is STRLEN. STRLEN is defined as MEM_SIZE and MEM_SIZE is /* Size_t: So it can be a signed integer. And overflow of signed variables is -- Petr |
From @tonycozOn Tue, Jan 06, 2015 at 06:39:02AM +0000, Petr Pisar wrote:
If available, Size_t is ANSI C89 size_t which is required to be You may be reading the (unlikely) alternatives incorrectly, they are MEM_SIZE_MAX is defined as ((MEM_SIZE)~0) (aka ((Size_t)~0) and Tony |
From @tonycozOn Mon, Jan 05, 2015 at 10:32:34PM -0800, Father Chrysostomos via RT wrote:
5.14 does the panic: $ perl -v | grep subversion While it says it's a panic, it's a croak, so it can be captured by The "Out of memory!" message is simple write() to stderr and a hard Tony |
From @tonycozOn Mon Jan 05 21:58:35 2015, tonyc wrote:
I've attached a format-patch patch instead, which directly checks against MEM_SIZE_MAX instead, rather than checking for the overflow. I considered making the comparison C<< newlen < MEM_SIZE_MAX >> instead, which produced the same code, in case (somehow!) MEM_SIZE/STRLEN/Size_t ended up signed, but since that would break MEM_SIZE_MAX anyway I though it would be slightly more confusing. As to the test, since we're allocating (address_space_size-1) bytes, the result creation should fail before any memory is allocated, so this isn't going to cause smokers (or other perl test runs) to attempt to allocate stupidly large amounts of space. Tony |
From @tonycoz0001-perl-123554-avoid-a-crash-from-SvGROW-MEM_SIZE_MAX.patchFrom 9a747d06187928e4f40916f7ecf1328aef95f84c Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 7 Jan 2015 11:09:15 +1100
Subject: [PATCH] [perl #123554] avoid a crash from SvGROW(MEM_SIZE_MAX)
---
sv.c | 7 +++++--
t/op/repeat.t | 6 +++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/sv.c b/sv.c
index fe092c4..11f8499 100644
--- a/sv.c
+++ b/sv.c
@@ -1594,8 +1594,11 @@ Perl_sv_grow(pTHX_ SV *const sv, STRLEN newlen)
* make more strings COW-able.
* If the new size is a big power of two, don't bother: we assume the
* caller wanted a nice 2^N sized block and will be annoyed at getting
- * 2^N+1 */
- if (newlen & 0xff)
+ * 2^N+1.
+ * Only increment if the allocation isn't MEM_SIZE_MAX,
+ * otherwise it will wrap to 0.
+ */
+ if (newlen & 0xff && newlen != MEM_SIZE_MAX)
newlen++;
#endif
diff --git a/t/op/repeat.t b/t/op/repeat.t
index 8df5241..421b705 100644
--- a/t/op/repeat.t
+++ b/t/op/repeat.t
@@ -6,7 +6,7 @@ BEGIN {
}
require './test.pl';
-plan(tests => 47);
+plan(tests => 49);
# compile time
@@ -173,3 +173,7 @@ for(($#that_array)x2) {
$_ *= 2;
}
is($#that_array, 28, 'list repetition propagates lvalue cx to its lhs');
+
+# see [perl #123554]
+ok(!eval '33x~3', "eval 33x~3 should panic, not crash perl");
+like($@, qr/^panic: memory wrap/, "check it's a panic");
--
1.7.10.4
|
From @cpansproutOn Mon Jan 05 10:23:49 2015, brian.carpenter@gmail.com wrote:
This particular test case: 3333333333333333333333333333333333333;33x~3333333333epl`t(/&/"oˇˇer web encodˇˇÄ |
|
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Wed Jan 07 03:50:46 2015, sprout wrote:
Something didn’t like the nulls that I unwittingly copied and pasted into the browser. So the message got cut off. This particular test case fails for the same reason as #123554. The syntax error makes no difference, because the 33x~3333333... is folded at compile time. -- Father Chrysostomos |
From @tonycozOn Tue Jan 06 16:23:36 2015, tonyc wrote:
Applied as fa8f4f8. Tony |
|
@tonycoz - Status changed from 'open' to 'resolved' |
From @geeknikI'm still attacking Perl5 with american fuzzy lop ( CC=/path/to/afl-gcc ./Configure This is perl 5, version 21, subversion 9 (v5.21.9 (v5.21.8-79-g4932eec)) Perl was compiled using all defaults (except adding -g to the CFLAGS). Program received signal SIGSEGV, Segmentation fault. gdb-peda$ bt gdb-peda$ exploit I noticed that this same test case crashes Perl 5.21.7 as well #0 __memcpy_ssse3_back () at The test case is a bit similar to the one in #123551, but the gdb output is |
From @geeknik |
From @tonycozOn Mon Feb 02 08:25:53 2015, brian.carpenter@gmail.com wrote:
I can't reproduce this in blead @ v5.21.8-186-g533686c (using afl-gcc): ./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-gcc -Doptimize=-O2\ -g && AFL_HARDEN=1 make -j6 test-prep $ AFL_HARDEN=1 valgrind -q ./perl ../123717.pl Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @geeknikWell that is what I get for using an outdated github repo. I've got this box pointed at the right repo now. We can probably close this if nobody can reproduce it. |
From @wolfsageOn Mon Feb 02 16:13:02 2015, brian.carpenter@gmail.com wrote:
This appears to have been fixed in passing by: commit fa8f4f8 [perl #123554] avoid a crash from SvGROW(MEM_SIZE_MAX) (Perhaps these tickets should be linked?) This appears to have been broken only recently - in perl-5.19.1, from: commit cbcb2a1 add 1 to SvGROW under COW (and fix svleak.t) -- Matthew Horsfall (alh) |
From @tonycozOn Sun Jan 18 20:48:57 2015, tonyc wrote:
With jhi's help, fixed a couple of other size overflows in 9efda33. Unfortunately, which perl isn't segfaulting, in some builds this falls back to the out of memory handler, which can't be caught by eval, so remove the test. Tony |
From @hvdsMerge into [perl #123554], it's the same issue. |
From @hvdsI think this is identical to [perl #123554], since the test case starts with '66x~6'. I'll merge. |
Migrated from rt.perl.org#123554 (status was 'resolved')
Searchable as RT123554$
The text was updated successfully, but these errors were encountered: