|
 |
1a4ac9 |
From c34e1dd29983e5d36d367462b9b4b4b8fcd5a0f8 Mon Sep 17 00:00:00 2001
|
|
|
1a4ac9 |
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
|
|
|
1a4ac9 |
Date: Mon, 6 Feb 2017 15:13:41 +0100
|
|
|
1a4ac9 |
Subject: [PATCH] Fix stack buffer overflow in deserialization of hooks.
|
|
|
1a4ac9 |
MIME-Version: 1.0
|
|
|
1a4ac9 |
Content-Type: text/plain; charset=UTF-8
|
|
|
1a4ac9 |
Content-Transfer-Encoding: 8bit
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
Ported from perl:
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
commit 3e998ddfb597cfae7bdb460b22e6c50440b1de92
|
|
|
1a4ac9 |
Author: John Lightsey <jd@cpanel.net>
|
|
|
1a4ac9 |
Date: Tue Jan 24 10:30:18 2017 -0600
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
Fix stack buffer overflow in deserialization of hooks.
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
The use of signed lengths resulted in a stack overflow in retrieve_hook()
|
|
|
1a4ac9 |
when a negative length was provided in the storable data.
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
The retrieve_blessed() codepath had a similar problem with the placement
|
|
|
1a4ac9 |
of the trailing null byte when negative lengths were provided.
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
Signed-off-by: Petr Písař <ppisar@redhat.com>
|
|
|
1a4ac9 |
---
|
|
|
1a4ac9 |
Storable.xs | 11 +++++++++--
|
|
|
1a4ac9 |
t/store.t | 12 +++++++++++-
|
|
|
1a4ac9 |
2 files changed, 20 insertions(+), 3 deletions(-)
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
diff --git a/Storable.xs b/Storable.xs
|
|
|
1a4ac9 |
index bc15d1d..3cce3ed 100644
|
|
|
1a4ac9 |
--- a/Storable.xs
|
|
|
1a4ac9 |
+++ b/Storable.xs
|
|
|
1a4ac9 |
@@ -4016,7 +4016,7 @@ static SV *retrieve_idx_blessed(pTHX_ stcxt_t *cxt, const char *cname)
|
|
|
1a4ac9 |
*/
|
|
|
1a4ac9 |
static SV *retrieve_blessed(pTHX_ stcxt_t *cxt, const char *cname)
|
|
|
1a4ac9 |
{
|
|
|
1a4ac9 |
- I32 len;
|
|
|
1a4ac9 |
+ U32 len;
|
|
|
1a4ac9 |
SV *sv;
|
|
|
1a4ac9 |
char buf[LG_BLESS + 1]; /* Avoid malloc() if possible */
|
|
|
1a4ac9 |
char *classname = buf;
|
|
|
1a4ac9 |
@@ -4037,6 +4037,9 @@ static SV *retrieve_blessed(pTHX_ stcxt_t *cxt, const char *cname)
|
|
|
1a4ac9 |
if (len & 0x80) {
|
|
|
1a4ac9 |
RLEN(len);
|
|
|
1a4ac9 |
TRACEME(("** allocating %d bytes for class name", len+1));
|
|
|
1a4ac9 |
+ if (len > I32_MAX) {
|
|
|
1a4ac9 |
+ CROAK(("Corrupted classname length"));
|
|
|
1a4ac9 |
+ }
|
|
|
1a4ac9 |
New(10003, classname, len+1, char);
|
|
|
1a4ac9 |
malloced_classname = classname;
|
|
|
1a4ac9 |
}
|
|
|
1a4ac9 |
@@ -4087,7 +4090,7 @@ static SV *retrieve_blessed(pTHX_ stcxt_t *cxt, const char *cname)
|
|
|
1a4ac9 |
*/
|
|
|
1a4ac9 |
static SV *retrieve_hook(pTHX_ stcxt_t *cxt, const char *cname)
|
|
|
1a4ac9 |
{
|
|
|
1a4ac9 |
- I32 len;
|
|
|
1a4ac9 |
+ U32 len;
|
|
|
1a4ac9 |
char buf[LG_BLESS + 1]; /* Avoid malloc() if possible */
|
|
|
1a4ac9 |
char *classname = buf;
|
|
|
1a4ac9 |
unsigned int flags;
|
|
|
1a4ac9 |
@@ -4221,6 +4224,10 @@ static SV *retrieve_hook(pTHX_ stcxt_t *cxt, const char *cname)
|
|
|
1a4ac9 |
else
|
|
|
1a4ac9 |
GETMARK(len);
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
+ if (len > I32_MAX) {
|
|
|
1a4ac9 |
+ CROAK(("Corrupted classname length"));
|
|
|
1a4ac9 |
+ }
|
|
|
1a4ac9 |
+
|
|
|
1a4ac9 |
if (len > LG_BLESS) {
|
|
|
1a4ac9 |
TRACEME(("** allocating %d bytes for class name", len+1));
|
|
|
1a4ac9 |
New(10003, classname, len+1, char);
|
|
|
1a4ac9 |
diff --git a/t/store.t b/t/store.t
|
|
|
1a4ac9 |
index be43299..1cbf021 100644
|
|
|
1a4ac9 |
--- a/t/store.t
|
|
|
1a4ac9 |
+++ b/t/store.t
|
|
|
1a4ac9 |
@@ -19,7 +19,7 @@ sub BEGIN {
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
use Storable qw(store retrieve store_fd nstore_fd fd_retrieve);
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
-use Test::More tests => 21;
|
|
|
1a4ac9 |
+use Test::More tests => 22;
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
$a = 'toto';
|
|
|
1a4ac9 |
$b = \$a;
|
|
|
1a4ac9 |
@@ -87,5 +87,15 @@ is(&dump($r), &dump(\%a));
|
|
|
1a4ac9 |
eval { $r = fd_retrieve(::OUT); };
|
|
|
1a4ac9 |
isnt($@, '');
|
|
|
1a4ac9 |
|
|
|
1a4ac9 |
+{
|
|
|
1a4ac9 |
+
|
|
|
1a4ac9 |
+ my $frozen =
|
|
|
1a4ac9 |
+ "\x70\x73\x74\x30\x04\x0a\x08\x31\x32\x33\x34\x35\x36\x37\x38\x04\x08\x08\x08\x03\xff\x00\x00\x00\x19\x08\xff\x00\x00\x00\x08\x08\xf9\x16\x16\x13\x16\x10\x10\x10\xff\x15\x16\x16\x16\x1e\x16\x16\x16\x16\x16\x16\x16\x16\x16\x16\x13\xf0\x16\x16\x16\xfe\x16\x41\x41\x41\x41\xe8\x03\x41\x41\x41\x41\x41\x41\x41\x41\x51\x41\xa9\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xb8\xac\xac\xac\xac\xac\xac\xac\xac\x9a\xac\xac\xac\xac\xac\xac\xac\xac\xac\x93\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\x00\x64\xac\xa8\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\xac\x2c\xac\x41\x41\x41\x41\x41\x41\x41\x41\x41\x00\x80\x41\x80\x41\x41\x41\x41\x41\x41\x51\x41\xac\xac\xac";
|
|
|
1a4ac9 |
+ open my $fh, '<', \$frozen;
|
|
|
1a4ac9 |
+ eval { Storable::fd_retrieve($fh); };
|
|
|
1a4ac9 |
+ pass('RT 130635: no stack smashing error when retrieving hook');
|
|
|
1a4ac9 |
+
|
|
|
1a4ac9 |
+}
|
|
|
1a4ac9 |
+
|
|
|
1a4ac9 |
close OUT or die "Could not close: $!";
|
|
|
1a4ac9 |
END { 1 while unlink 'store' }
|
|
|
1a4ac9 |
--
|
|
|
1a4ac9 |
2.7.4
|
|
|
1a4ac9 |
|