PERL-5.26.1 heap-buffer-overflow READ of size 11 #16343
Comments
From sraums2498@gmail.com================================================================= 0x60200000d91a is located 0 bytes to the right of 10-byte region SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 __interceptor_strlen -- |
From sraums2498@gmail.com |
From @hvdsThis looks very similar to rt132654: I'll add a cross-reference, but it's a bit too early to merge them. Hugo |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Tue, 09 Jan 2018 00:46:06 -0800, hv wrote:
This can be further simplified to: ./miniperl -e '0 * unpack "u", "ab"'
It's unrelated - 132654 is about (mis-)use of the p pack format. The problem here is the "u" decoder, which creates a new SV (upgraded to SVt_PV in the case that matters) and sets POK on it. When the "ab" fails to decode, no changes are made to the PV, leaving it unterminated. pp_add (or pp_multiply, depending on whether you use Hugo's or the original's test case) then tries to use that SV as a number, eventually leading to sv_2iv_flags trying to decode the (unterminated) PV as a number via Perl_my_atof() which treats the PV as a C NUL terminated string and accesses the uninitialized bytes. Trivially fixed by ensuring the PV is NUL terminated. Inline Patchdiff --git a/pp_pack.c b/pp_pack.c
index 8937d6d715..50e9b30221 100644
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -1710,7 +1710,10 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
if (!checksum) {
const STRLEN l = (STRLEN) (strend - s) * 3 / 4;
sv = sv_2mortal(newSV(l));
- if (l) SvPOK_on(sv);
+ if (l) {
+ SvPOK_on(sv);
+ *SvEND(sv) = '\0';
+ }
}
/* Note that all legal uuencoded strings are ASCII printables, so
I think it's highly likely there's code accepting untrusted uuencoded data and passing it through C<unpack "u">. There's a (low) chance of an attacker who can control the uudecoded data causing perl to SEGV if the strlen happens to run off the end of the mapped memory block. This should only cause perl to crash, there's nothing that will write to the memory block. This issue can only occur if the first byte in the string is not a uuencoding character - so no decoded data is emitted, so the attacker has zero control over what ends up the SV that's causing the problem. In the past we've treated similar issues as *not* being security issues. I don't think this is a security issue. Tony |
From @iabynOn Mon, Jan 22, 2018 at 09:14:09PM -0800, Tony Cook via RT wrote:
Agreed. I think you should go ahead apply the patch. -- |
From @geeknikThis overflow was triggered while fuzzing Perl v5.28.0-RC2-2-g197e798, echo "dW5wYWNrInV1LwAiLCQw" | base64 -d | tee test099.pl ; ./perl test099.pl ==22268==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000f5a is located 0 bytes to the right of 10-byte region SUMMARY: AddressSanitizer: heap-buffer-overflow |
From @iabynOn Mon, Jun 25, 2018 at 12:11:33PM -0700, Brian Carpenter wrote:
Or more prozaically, $ valgrind ./perl -e'unpack "uu/\0", "abc";' The direct fault is that the 'u' action is creating an SV with a I'm not yet sure how serious it is. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @khwilliamsonOn 06/25/2018 05:07 PM, Dave Mitchell wrote:
See also |
From @iabynOn Tue, Jun 26, 2018 at 12:07:36AM +0100, Dave Mitchell wrote:
In general, unpack('u', 'nonlegal-uuencode-chars') will return 0.0 + unpack("u", "abc"); trips up in atof(). It's very plausible that an attacker can provide such malformed uuencoded Typically you would expect the result of unpack('u') to be fed to My example above of treating the uudecoded result as a number is, I think, So I think I should just fix it (the fix is trivial) and not treat it as a -- |
From @tonycozOn Wed, 27 Jun 2018 01:37:07 -0700, davem wrote:
I believe this is the same issue as #132655. Tony |
From @tonycozOn Mon, 09 Apr 2018 08:53:03 -0700, davem wrote:
Here's the patch, I'll apply it in a couple of days, unless someone suggests we make this a security issue. Tony |
From @tonycoz0001-perl-132655-nul-terminate-result-of-unpack-u-of-inva.patchFrom 49bb9eff715af452bcbeb2f94716891d185a0f54 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Mon, 20 Aug 2018 16:31:45 +1000
Subject: (perl #132655) nul terminate result of unpack "u" of invalid data
In the given test case, Perl_atof2() would run off the end of the PV,
producing an error from ASAN.
---
pp_pack.c | 5 ++++-
t/op/pack.t | 9 ++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/pp_pack.c b/pp_pack.c
index 5e9cc64301..f8be9d48ae 100644
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -1727,7 +1727,10 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
if (!checksum) {
const STRLEN l = (STRLEN) (strend - s) * 3 / 4;
sv = sv_2mortal(newSV(l));
- if (l) SvPOK_on(sv);
+ if (l) {
+ SvPOK_on(sv);
+ *SvEND(sv) = '\0';
+ }
}
/* Note that all legal uuencoded strings are ASCII printables, so
diff --git a/t/op/pack.t b/t/op/pack.t
index cf0e286509..bb9f865091 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 => 14717;
+plan tests => 14718;
use strict;
use warnings qw(FATAL all);
@@ -2081,3 +2081,10 @@ SKIP:
fresh_perl_like('pack "c10f1073741824"', qr/Out of memory during pack/, { stderr => 1 },
"integer overflow calculating allocation (multiply)");
}
+
+{
+ # [perl #132655] heap-buffer-overflow READ of size 11
+ # only expect failure under ASAN (and maybe valgrind)
+ fresh_perl_is('0.0 + unpack("u", "ab")', "", { stderr => 1 },
+ "ensure unpack u of invalid data nul terminates result");
+}
--
2.11.0
|
From hanno@hboeck.deHi, An out of bounds heap read can be triggered in perl with a call to perl -e 'unpack("u/X", "r0");' It will read 11 bytes beyond a defined buffer. This can be detected by compiling perl with address sanitizer. Works ==26194==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000165a at pc 0x00000044b1fb bp 0x7ffedc131b90 sp 0x7ffedc131338 0x60200000165a is located 0 bytes to the right of 10-byte region [0x602000001650,0x60200000165a) SUMMARY: AddressSanitizer: heap-buffer-overflow (/r/perl/perl+0x44b1fa) in __interceptor_strlen -- mail/jabber: hanno@hboeck.de |
From @tonycozOn Sun, 19 Aug 2018 23:32:55 -0700, tonyc wrote:
More than a couple of days. Applied as 12cad9b. This ticket is now public. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @tonycozOn Tue, 21 Aug 2018 04:19:15 -0700, hanno@hboeck.de wrote:
This is a duplicate of 132655, and the fix, 12cad9b, also fixes this. Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Sun, 19 Aug 2018 22:18:33 -0700, tonyc wrote:
The fix for 132655 also fixes this, merging into 132655. Tony |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.30.0, this and 160 other issues have been Perl 5.30.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#132655 (status was 'resolved')
Searchable as RT132655$
The text was updated successfully, but these errors were encountered: