Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upheap-buffer-overflow S_pack_Rec pp_pack.c:3108 #15572
Comments
From @geeknikThis one crashes v5.25.5 (v5.25.4-25-g109ac34*) with ASAN, however a hexdump -C over307 ==18296==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300000e8d8 is located 0 bytes to the right of 24-byte region SUMMARY: AddressSanitizer: heap-buffer-overflow /root/perl/pp_pack.c:3108 |
From @geeknik |
From @dcollinsnApologies if this is a dupe. This appears to be a legitimate bug in pack, perl -e 'pack SWFW, 0,0,0,-1' (gdb) run Program received signal SIGABRT, Aborted. However, I also decided to valgrind this, and got an invalid write even Use of code point 0xFFFFFFFFFFFFFFFF is deprecated; the permissible max is I popped in libdislocator to turn that into a segfault, and ran GDB again: $ This is free software: you are free to change and redistribute it. Program received signal SIGSEGV, Segmentation fault. This /seems/ to just be an off-by-one error - AFL crash explorer hasn't |
From @geeknikperl -e 'pack~~W9,\0,\0,0,\0' triggers this as well in v5.25.4-90-g5b549d1. ==23676==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x604000009a78 is located 0 bytes to the right of 40-byte region SUMMARY: AddressSanitizer: heap-buffer-overflow |
From @tonycozOn Wed Aug 31 01:40:20 2016, brian.carpenter@gmail.com wrote:
The attached fixes this for me. This will only ever overflow by one byte, so I could see it causing a Tony |
From @tonycoz0001-perl-129149-avoid-a-heap-buffer-overflow-with-pack-W.patchFrom ab19876f2d2aea3a1829dee139b4a1b816f09681 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 7 Sep 2016 16:51:39 +1000
Subject: (perl #129149) avoid a heap buffer overflow with pack "W"...
---
pp_pack.c | 2 +-
t/op/pack.t | 13 ++++++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/pp_pack.c b/pp_pack.c
index 40c3100..09d91a5 100644
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -2581,7 +2581,7 @@ S_pack_rec(pTHX_ SV *cat, tempsym_t* symptr, SV **beglist, SV **endlist )
if (in_bytes) auv = auv % 0x100;
if (utf8) {
W_utf8:
- if (cur > end) {
+ if (cur >= end) {
*cur = '\0';
SvCUR_set(cat, cur - start);
diff --git a/t/op/pack.t b/t/op/pack.t
index df16464..7ec09ae 100644
--- a/t/op/pack.t
+++ b/t/op/pack.t
@@ -12,7 +12,7 @@ my $no_endianness = $] > 5.009 ? '' :
my $no_signedness = $] > 5.009 ? '' :
"Signed/unsigned pack modifiers not available on this perl";
-plan tests => 14712;
+plan tests => 14713;
use strict;
use warnings qw(FATAL all);
@@ -2049,3 +2049,14 @@ ok(1, "argument underflow did not crash");
is(pack("H40", $up_nul), $twenty_nuls,
"check pack H zero fills (utf8 source)");
}
+
+{
+ # [perl #129149] the code below would write one past the end of the output
+ # buffer, only detected by ASAN, not by valgrind
+ $Config{ivsize} >= 8
+ or skip "[perl #129149] need 64-bit for this test", 1;
+ fresh_perl_is(<<'EOS', "ok\n", { stderr => 1 }, "pack W overflow");
+print pack("ucW", "0000", 0, 140737488355327) eq "\$,#`P,```\n\0\x{7fffffffffff}"
+ ? "ok\n" : "not ok\n";
+EOS
+}
--
2.1.4
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Sat Sep 03 07:47:23 2016, dcollinsn@gmail.com wrote:
This looks like the same bug as security ticket 129149, which I posted Tony |
From @tonycoz0001-perl-129149-avoid-a-heap-buffer-overflow-with-pack-W.patchFrom ab19876f2d2aea3a1829dee139b4a1b816f09681 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 7 Sep 2016 16:51:39 +1000
Subject: (perl #129149) avoid a heap buffer overflow with pack "W"...
---
pp_pack.c | 2 +-
t/op/pack.t | 13 ++++++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/pp_pack.c b/pp_pack.c
index 40c3100..09d91a5 100644
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -2581,7 +2581,7 @@ S_pack_rec(pTHX_ SV *cat, tempsym_t* symptr, SV **beglist, SV **endlist )
if (in_bytes) auv = auv % 0x100;
if (utf8) {
W_utf8:
- if (cur > end) {
+ if (cur >= end) {
*cur = '\0';
SvCUR_set(cat, cur - start);
diff --git a/t/op/pack.t b/t/op/pack.t
index df16464..7ec09ae 100644
--- a/t/op/pack.t
+++ b/t/op/pack.t
@@ -12,7 +12,7 @@ my $no_endianness = $] > 5.009 ? '' :
my $no_signedness = $] > 5.009 ? '' :
"Signed/unsigned pack modifiers not available on this perl";
-plan tests => 14712;
+plan tests => 14713;
use strict;
use warnings qw(FATAL all);
@@ -2049,3 +2049,14 @@ ok(1, "argument underflow did not crash");
is(pack("H40", $up_nul), $twenty_nuls,
"check pack H zero fills (utf8 source)");
}
+
+{
+ # [perl #129149] the code below would write one past the end of the output
+ # buffer, only detected by ASAN, not by valgrind
+ $Config{ivsize} >= 8
+ or skip "[perl #129149] need 64-bit for this test", 1;
+ fresh_perl_is(<<'EOS', "ok\n", { stderr => 1 }, "pack W overflow");
+print pack("ucW", "0000", 0, 140737488355327) eq "\$,#`P,```\n\0\x{7fffffffffff}"
+ ? "ok\n" : "not ok\n";
+EOS
+}
--
2.1.4
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Tue, Sep 06, 2016 at 11:54:34PM -0700, Tony Cook via RT wrote:
This fix looks good to me. An attacker would have to be in a position I think the fix should be pushed to blead. -- |
From @dur-randirCreated by @dur-randirWhile fuzzing perl v5.25.8-207-gcbe2fc5001 built with afl and run pack"Z*WWW",1.01E50,0,0,1E20 to perform an access outside of an allocated memory slot. ASAN diagnostics are: % ./perl /tmp/0001
|
From @tonycozOn Tue, 06 Dec 2016 09:18:27 -0800, davem wrote:
I've made the ticket public and pushed the patch as bf4a926. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @tonycozOn Sat, 14 Jan 2017 10:29:27 -0800, randir wrote:
This looks like a duplicate of #129149 and my patch for that prevents the crash. Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @bulk88On Mon, 16 Jan 2017 15:49:16 -0800, tonyc wrote:
Your patch is failing on 32 bit windows. Your skip() has no SKIP: in the patch. ok 14713 # skip [perl #129149] need 64-bit for this test Test Summary Report op/pack.t (Wstat: 65280 Tests: 14713 Failed: 0) Label not found for "last SKIP" at ./test.pl line 518. -- |
From @tonycozOn Mon, Jan 16, 2017 at 08:15:39PM -0800, bulk88 via RT wrote:
Thanks, fixed in 30be69c. Tony |
From @tonycozOn Wed, 07 Sep 2016 17:44:54 -0700, tonyc wrote:
Which has been fixed and your case no longer fails. Merged your 129187 into 129149 (which is public.) Tony |
From @tonycozOn Mon, 16 Jan 2017 15:58:07 -0800, tonyc wrote:
No dissent, so merging into the (closed) 129149. Tony |
From @khwilliamsonThank 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 Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#129149 (status was 'resolved')
Searchable as RT129149$