From c34e1dd29983e5d36d367462b9b4b4b8fcd5a0f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
Date: Mon, 6 Feb 2017 15:13:41 +0100
Subject: [PATCH] Fix stack buffer overflow in deserialization of hooks.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ported from perl:

commit 3e998ddfb597cfae7bdb460b22e6c50440b1de92
Author: John Lightsey <jd@cpanel.net>
Date:   Tue Jan 24 10:30:18 2017 -0600

    Fix stack buffer overflow in deserialization of hooks.

    The use of signed lengths resulted in a stack overflow in retrieve_hook()
    when a negative length was provided in the storable data.

    The retrieve_blessed() codepath had a similar problem with the placement
    of the trailing null byte when negative lengths were provided.

Signed-off-by: Petr Písař <ppisar@redhat.com>
---
 Storable.xs | 11 +++++++++--
 t/store.t   | 12 +++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Storable.xs b/Storable.xs
index bc15d1d..3cce3ed 100644
--- a/Storable.xs
+++ b/Storable.xs
@@ -4016,7 +4016,7 @@ static SV *retrieve_idx_blessed(pTHX_ stcxt_t *cxt, const char *cname)
  */
 static SV *retrieve_blessed(pTHX_ stcxt_t *cxt, const char *cname)
 {
-	I32 len;
+	U32 len;
 	SV *sv;
 	char buf[LG_BLESS + 1];		/* Avoid malloc() if possible */
 	char *classname = buf;
@@ -4037,6 +4037,9 @@ static SV *retrieve_blessed(pTHX_ stcxt_t *cxt, const char *cname)
 	if (len & 0x80) {
 		RLEN(len);
 		TRACEME(("** allocating %d bytes for class name", len+1));
+		if (len > I32_MAX) {
+			CROAK(("Corrupted classname length"));
+		}
 		New(10003, classname, len+1, char);
 		malloced_classname = classname;
 	}
@@ -4087,7 +4090,7 @@ static SV *retrieve_blessed(pTHX_ stcxt_t *cxt, const char *cname)
  */
 static SV *retrieve_hook(pTHX_ stcxt_t *cxt, const char *cname)
 {
-	I32 len;
+	U32 len;
 	char buf[LG_BLESS + 1];		/* Avoid malloc() if possible */
 	char *classname = buf;
 	unsigned int flags;
@@ -4221,6 +4224,10 @@ static SV *retrieve_hook(pTHX_ stcxt_t *cxt, const char *cname)
 		else
 			GETMARK(len);
 
+		if (len > I32_MAX) {
+			CROAK(("Corrupted classname length"));
+		}
+
 		if (len > LG_BLESS) {
 			TRACEME(("** allocating %d bytes for class name", len+1));
 			New(10003, classname, len+1, char);
diff --git a/t/store.t b/t/store.t
index be43299..1cbf021 100644
--- a/t/store.t
+++ b/t/store.t
@@ -19,7 +19,7 @@ sub BEGIN {
 
 use Storable qw(store retrieve store_fd nstore_fd fd_retrieve);
 
-use Test::More tests => 21;
+use Test::More tests => 22;
 
 $a = 'toto';
 $b = \$a;
@@ -87,5 +87,15 @@ is(&dump($r), &dump(\%a));
 eval { $r = fd_retrieve(::OUT); };
 isnt($@, '');
 
+{
+
+    my $frozen =
+      "\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";
+    open my $fh, '<', \$frozen;
+    eval { Storable::fd_retrieve($fh); };
+    pass('RT 130635:  no stack smashing error when retrieving hook');
+
+}
+
 close OUT or die "Could not close: $!";
 END { 1 while unlink 'store' }
-- 
2.7.4