Skip to content

Commit c6f7713

Browse files
arcReini Urban
authored and
Reini Urban
committed
Storable: throw exception on huge values
The Storable data format is incapable of representing lengths of 2**31 or greater; and if you try, you can get segfaults or corrupt data or other fun and games. Though it would be undeniably good to fix this properly, this is just a simple starting point: the limitation is documented, and an exception is thrown when such data is encountered. Signed-off-by: Reini Urban <[email protected]>
1 parent 9370eae commit c6f7713

File tree

3 files changed

+75
-2
lines changed

3 files changed

+75
-2
lines changed

Storable.pm

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ package Storable; @ISA = qw(Exporter);
2424

2525
use vars qw($canonical $forgive_me $VERSION);
2626

27-
$VERSION = '2.53_03';
27+
$VERSION = '2.57_01';
2828

2929
BEGIN {
3030
if (eval { local $SIG{__DIE__}; require Log::Agent; 1 }) {
@@ -1178,7 +1178,7 @@ A hash with 2**31 or more keys
11781178
11791179
=back
11801180
1181-
Attempting to do so will result in unpredicatable overflow results.
1181+
Attempting to do so will yield an exception.
11821182
11831183
This may be fixed in the future.
11841184

Storable.xs

+13
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,13 @@ static const char byteorderstr_56[] = {BYTEORDER_BYTES_56, 0};
875875
#define PL_sv_placeholder PL_sv_undef
876876
#endif
877877

878+
#define MUST_FIT_IN_I32(x) \
879+
STMT_START { \
880+
if ((UV)(x) > (UV)0x7fffffffu) { \
881+
CROAK(("Storable cannot yet handle data that needs a 64-bit machine")); \
882+
} \
883+
} STMT_END
884+
878885
/*
879886
* Useful store shortcuts...
880887
*/
@@ -941,6 +948,7 @@ static const char byteorderstr_56[] = {BYTEORDER_BYTES_56, 0};
941948
if (len) \
942949
WRITE(pv, len); \
943950
} else { \
951+
MUST_FIT_IN_I32(len); \
944952
PUTMARK(large); \
945953
WLEN(len); \
946954
WRITE(pv, len); \
@@ -2259,6 +2267,7 @@ static int store_scalar(pTHX_ stcxt_t *cxt, SV *sv)
22592267
}
22602268
#endif
22612269

2270+
MUST_FIT_IN_I32(len);
22622271
wlen = (I32) len; /* WLEN via STORE_SCALAR expects I32 */
22632272
if (SvUTF8 (sv))
22642273
STORE_UTF8STR(pv, wlen);
@@ -2288,6 +2297,8 @@ static int store_array(pTHX_ stcxt_t *cxt, AV *av)
22882297
I32 i;
22892298
int ret;
22902299

2300+
MUST_FIT_IN_I32(av_len(av) + 1);
2301+
22912302
TRACEME(("store_array (0x%"UVxf")", PTR2UV(av)));
22922303

22932304
/*
@@ -2389,6 +2400,8 @@ static int store_hash(pTHX_ stcxt_t *cxt, HV *hv)
23892400
) ? 1 : 0);
23902401
unsigned char hash_flags = (SvREADONLY(hv) ? SHV_RESTRICTED : 0);
23912402

2403+
MUST_FIT_IN_I32(HvTOTALKEYS(hv));
2404+
23922405
if (flagged_hash) {
23932406
/* needs int cast for C++ compilers, doesn't it? */
23942407
TRACEME(("store_hash (0x%"UVxf") (flags %x)", PTR2UV(hv),

t/huge.t

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#!./perl
2+
3+
use strict;
4+
use warnings;
5+
6+
use Config;
7+
use Storable qw(dclone);
8+
use Test::More;
9+
10+
BEGIN {
11+
plan skip_all => 'Storable was not built'
12+
if $ENV{PERL_CORE} && $Config{'extensions'} !~ /\b Storable \b/x;
13+
plan skip_all => 'Need 64-bit pointers for this test'
14+
if $Config{ptrsize} < 8;
15+
plan skip_all => 'Need ~4 GiB of core for this test'
16+
if !$ENV{PERL_TEST_MEMORY} || $ENV{PERL_TEST_MEMORY} < 4;
17+
}
18+
19+
# Just too big to fit in an I32.
20+
my $huge = int(2 ** 31);
21+
22+
# For now, all of these should throw an exception. Actually storing and
23+
# retrieving them would require changing the serialisation format, and
24+
# that's a larger task than I'm looking to undertake right now.
25+
my @cases = (
26+
['huge string',
27+
sub { my $s = 'x' x $huge; \$s }],
28+
29+
['huge array',
30+
sub { my @x; $x[$huge] = undef; \@x }],
31+
32+
['array with huge element',
33+
sub { my $s = 'x' x $huge; [$s] }],
34+
35+
# A hash with a huge number of keys would require tens of gigabytes of
36+
# memory, which doesn't seem like a good idea even for this test file.
37+
38+
['hash with huge value',
39+
sub { my $s = 'x' x $huge; +{ foo => $s } }],
40+
41+
# Can't test hash with a huge key, because Perl internals currently
42+
# limit hash keys to <2**31 anyway
43+
);
44+
45+
plan tests => scalar @cases;
46+
47+
for (@cases) {
48+
my ($desc, $build) = @$_;
49+
note "building test input: $desc";
50+
my $input = $build->();
51+
note "running test: $desc";
52+
my ($exn, $clone);
53+
$exn = $@ if !eval { $clone = dclone($input); 1 };
54+
like($exn, qr/^Storable cannot yet handle data that needs a 64-bit machine\b/,
55+
"$desc: throw an exception, not a segfault or panic");
56+
57+
# Ensure the huge objects are freed right now:
58+
undef $input;
59+
undef $clone;
60+
}

0 commit comments

Comments
 (0)