SDBM Memory Safety Issues #16164
Comments
From CYoung@tripwire.comI have identified a test case which triggers a memory safety issue (as detected by ASAN) within the SDBM_File module. The following crafted pag data can reproduce this issue: To reproduce, base64 decode the above data into a file (demo.dbm.pag), create an additional empty file (demo.dbm.dir), and run the following script using a perl binary instrumented with AddressSanitizer (-fsanitize=address): use warnings; my %dbm; tie %dbm, 'SDBM_File', $db_file, O_RDWR, 0; my $value=$dbm{'foo'}; foreach (sort keys(%dbm)) { The result of running this script on my build (5.26.1-RC1) is the following ASAN report of a heap-buffer-overflow due to a wild memmove:
|
From @tonycozOn Fri, 22 Sep 2017 16:16:03 -0700, CYoung@tripwire.com wrote:
While this is a bug, I'm not sure if it's a security issue. SDBM files aren't portable across architectures - differences in endianess trips that up if nothing else - I don't know if we support using them as some sort of interchange between untrusted actors. Is it possible to produce such a "bad" .pag file through normal use of a SDBM_File tied variable? Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From CYoung@tripwire.comYes, you have a valid point that SDBM may not generally be used to exchange data with untrusted actors but I do still believe that this should be treated as a security issue. Perl being so ubiquitous, it is likely that there are some projects which do handle SDBM from untrusted sources. It’s worth noting that in Apache, where code forked from Perl is used for the APR SDBM implementation, the password database can be stored in SDBM format. This means that for a shared hosting environment, Apache could realistically be processing a crafted password database from one webmaster while also handling user requests for a site operated by a different user. I would also like to point out that the Perl docs do not warn users against processing SDBM from an untrusted source as is the case with for example Storable. Best Regards, On 9/24/17, 9:27 PM, "Tony Cook via RT" <perl5-security-report-followup@perl.org> wrote: On Fri, 22 Sep 2017 16:16:03 -0700, CYoung@tripwire.com wrote: |
From @iabynOn Mon, Sep 25, 2017 at 02:25:28PM +0000, Craig Young wrote:
I think it would take considerable effort to audit and fix the sdbm code
So instead, I propose that we add a warning to the pod file (and maybe Here's some suggested text to be added just before 'BUGS AND WARNINGS': =head1 SECURITY WARNING B<Do not accept sbdm files from untrusted sources!> The sdbm file format was designed for speed and convenience, not for -- |
From CYoung@tripwire.comThis sounds good. On Mon, Sep 25, 2017 at 02:25:28PM +0000, Craig Young wrote:
I think it would take considerable effort to audit and fix the sdbm code
So instead, I propose that we add a warning to the pod file (and maybe Here's some suggested text to be added just before 'BUGS AND WARNINGS': =head1 SECURITY WARNING B<Do not accept sbdm files from untrusted sources!> The sdbm file format was designed for speed and convenience, not for -- |
From @tonycozOn Wed, 29 Nov 2017 03:18:41 -0800, davem wrote:
As a patch, including the other DBM_Files The non-SDBM_File versions are kind of vague/weasel-wordy, since we don't supply their underlying implementations. If anyone has a link to GDBM being safe with untrusted files, I'd appreciate a link, I couldn't find anything with Google. Tony |
From @tonycozsdbm_file.patchFrom 32c02e1cee725afe336a23d1bb42197a8f50718c Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 24 Jan 2018 15:03:39 +1100
Subject: [PATCH 1/2] (perl #132147) add security warnings to the *DBM_File
modules
---
ext/GDBM_File/GDBM_File.pm | 12 ++++++++++++
ext/NDBM_File/NDBM_File.pm | 17 +++++++++++++++++
ext/ODBM_File/ODBM_File.pm | 17 +++++++++++++++++
ext/SDBM_File/SDBM_File.pm | 8 ++++++++
4 files changed, 54 insertions(+)
diff --git a/ext/GDBM_File/GDBM_File.pm b/ext/GDBM_File/GDBM_File.pm
index a33b8b59b1..fe34470bd3 100644
--- a/ext/GDBM_File/GDBM_File.pm
+++ b/ext/GDBM_File/GDBM_File.pm
@@ -31,6 +31,18 @@ C<ftp.gnu.org>, but you are strongly urged to use one of the many
mirrors. You can obtain a list of mirror sites from
L<http://www.gnu.org/order/ftp.html>.
+=head1 SECURITY AND PORTABILITY
+
+B<Do not accept GDBM files from untrusted sources.>
+
+GDBM files are not portable across platforms.
+
+The GDBM documentation doesn't imply that files from untrusted sources
+can be safely used with C<libgdbm>.
+
+A maliciously crafted file might cause perl to crash or even expose a
+security vulnerability.
+
=head1 BUGS
The available functions and the gdbm/perl interface need to be documented.
diff --git a/ext/NDBM_File/NDBM_File.pm b/ext/NDBM_File/NDBM_File.pm
index fc250ec840..97c3917c92 100644
--- a/ext/NDBM_File/NDBM_File.pm
+++ b/ext/NDBM_File/NDBM_File.pm
@@ -104,6 +104,23 @@ This warning is emitted when you try to store a key or a value that
is too long. It means that the change was not recorded in the
database. See BUGS AND WARNINGS below.
+=head1 SECURITY AND PORTABILITY
+
+B<Do not accept NDBM files from untrusted sources.>
+
+On modern Linux systems these are typically GDBM files, which are not
+portable across platforms.
+
+The GDBM documentation doesn't imply that files from untrusted sources
+can be safely used with C<libgdbm>.
+
+Systems that don't use GDBM compatibilty for ndbm support will be
+using a platform specific library, possibly inherited from BSD
+systems, where it may or may not be safe to use an untrusted file.
+
+A maliciously crafted file might cause perl to crash or even expose a
+security vulnerability.
+
=head1 BUGS AND WARNINGS
There are a number of limits on the size of the data that you can
diff --git a/ext/ODBM_File/ODBM_File.pm b/ext/ODBM_File/ODBM_File.pm
index 99799bc520..6d89a229f6 100644
--- a/ext/ODBM_File/ODBM_File.pm
+++ b/ext/ODBM_File/ODBM_File.pm
@@ -101,6 +101,23 @@ This warning is emitted when you try to store a key or a value that
is too long. It means that the change was not recorded in the
database. See BUGS AND WARNINGS below.
+=head1 SECURITY AND PORTABILITY
+
+B<Do not accept ODBM files from untrusted sources.>
+
+On modern Linux systems these are typically GDBM files, which are not
+portable across platforms.
+
+The GDBM documentation doesn't imply that files from untrusted sources
+can be safely used with C<libgdbm>.
+
+Systems that don't use GDBM compatibilty for old dbm support will be
+using a platform specific library, possibly inherited from BSD
+systems, where it may or may not be safe to use an untrusted file.
+
+A maliciously crafted file might cause perl to crash or even expose a
+security vulnerability.
+
=head1 BUGS AND WARNINGS
There are a number of limits on the size of the data that you can
diff --git a/ext/SDBM_File/SDBM_File.pm b/ext/SDBM_File/SDBM_File.pm
index 5df9085760..7be9263417 100644
--- a/ext/SDBM_File/SDBM_File.pm
+++ b/ext/SDBM_File/SDBM_File.pm
@@ -119,6 +119,14 @@ This warning is emitted when you try to store a key or a value that
is too long. It means that the change was not recorded in the
database. See BUGS AND WARNINGS below.
+=head1 SECURITY WARNING
+
+B<Do not accept SDBM files from untrusted sources!>
+
+The sdbm file format was designed for speed and convenience, not for
+portability or security. A maliciously crafted file might cause perl to
+crash or even expose a security vulnerability.
+
=head1 BUGS AND WARNINGS
There are a number of limits on the size of the data that you can
--
2.11.0
From 364f33500c0ac3b4cc0aa114c5f3960e519a9a99 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 24 Jan 2018 15:10:10 +1100
Subject: [PATCH 2/2] (perl #132147) bump *DBM_File versions
---
ext/GDBM_File/GDBM_File.pm | 2 +-
ext/NDBM_File/NDBM_File.pm | 2 +-
ext/ODBM_File/ODBM_File.pm | 2 +-
ext/SDBM_File/SDBM_File.pm | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ext/GDBM_File/GDBM_File.pm b/ext/GDBM_File/GDBM_File.pm
index fe34470bd3..b4fc49f42e 100644
--- a/ext/GDBM_File/GDBM_File.pm
+++ b/ext/GDBM_File/GDBM_File.pm
@@ -85,7 +85,7 @@ require XSLoader;
);
# This module isn't dual life, so no need for dev version numbers.
-$VERSION = '1.17';
+$VERSION = '1.18';
XSLoader::load();
diff --git a/ext/NDBM_File/NDBM_File.pm b/ext/NDBM_File/NDBM_File.pm
index 97c3917c92..ead745da24 100644
--- a/ext/NDBM_File/NDBM_File.pm
+++ b/ext/NDBM_File/NDBM_File.pm
@@ -7,7 +7,7 @@ require Tie::Hash;
require XSLoader;
our @ISA = qw(Tie::Hash);
-our $VERSION = "1.14";
+our $VERSION = "1.15";
XSLoader::load();
diff --git a/ext/ODBM_File/ODBM_File.pm b/ext/ODBM_File/ODBM_File.pm
index 6d89a229f6..7bdbecc73c 100644
--- a/ext/ODBM_File/ODBM_File.pm
+++ b/ext/ODBM_File/ODBM_File.pm
@@ -7,7 +7,7 @@ require Tie::Hash;
require XSLoader;
our @ISA = qw(Tie::Hash);
-our $VERSION = "1.15";
+our $VERSION = "1.16";
XSLoader::load();
diff --git a/ext/SDBM_File/SDBM_File.pm b/ext/SDBM_File/SDBM_File.pm
index 7be9263417..30e380a6bb 100644
--- a/ext/SDBM_File/SDBM_File.pm
+++ b/ext/SDBM_File/SDBM_File.pm
@@ -7,7 +7,7 @@ require Tie::Hash;
require XSLoader;
our @ISA = qw(Tie::Hash);
-our $VERSION = "1.14";
+our $VERSION = "1.15";
our @EXPORT_OK = qw(PAGFEXT DIRFEXT PAIRMAX);
use Exporter "import";
--
2.11.0
|
From @iabynOn Tue, Jan 23, 2018 at 08:21:54PM -0800, Tony Cook via RT wrote:
Looks fine to me.
No idea. -- |
From @tonycozOn Sun, 04 Nov 2018 15:38:33 -0800, tonyc wrote:
The attached patches test for the issues from this ticket and 132151 (first patch), fixes those issues in particular (second patch), and adds tests and fixes for issues found by fuzz testing, and also hardens chkpage() beyond that (third patch). Tony |
From @tonycoz0001-perl-132147-add-tests-for-corrupt-files-from-tickets.patchFrom e554575e86a57188af92026d46a95f8ea3f1fd1f Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 6 Nov 2018 14:12:53 +1100
Subject: (perl #132147) add tests for corrupt files from tickets
---
MANIFEST | 1 +
ext/SDBM_File/t/corrupt.t | 86 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+)
create mode 100644 ext/SDBM_File/t/corrupt.t
diff --git a/MANIFEST b/MANIFEST
index f05629faef..7d8aa069f1 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -4211,6 +4211,7 @@ ext/SDBM_File/sdbm.h SDBM kit
ext/SDBM_File/SDBM_File.pm SDBM extension Perl module
ext/SDBM_File/SDBM_File.xs SDBM extension external subroutines
ext/SDBM_File/t/constants.t See if SDBM_File constants work
+ext/SDBM_File/t/corrupt.t See if SDBM_File handles corrupt files
ext/SDBM_File/t/prep.t See if SDBM_File with extra argument works
ext/SDBM_File/t/sdbm.t See if SDBM_File works
ext/SDBM_File/tune.h SDBM kit
diff --git a/ext/SDBM_File/t/corrupt.t b/ext/SDBM_File/t/corrupt.t
new file mode 100644
index 0000000000..b4e41aab64
--- /dev/null
+++ b/ext/SDBM_File/t/corrupt.t
@@ -0,0 +1,86 @@
+#!./perl
+use strict;
+use Test::More;
+use MIME::Base64;
+use File::Temp 'tempfile';
+use SDBM_File;
+use Fcntl qw(O_RDWR);
+use Errno qw(EINVAL);
+
+my ($dirfh, $dirname) = tempfile(UNLINK => 1);
+my ($pagfh, $pagname) = tempfile(UNLINK => 1);
+close $dirfh;
+close pagefh;
+
+while (my $testdata = do { local $/ = "END\n"; <DATA>; }) {
+ my ($note, $base64) = $testdata =~ /\A([^\n]+)\n(.*)/s
+ or die;
+ my $raw = decode_base64($base64);
+ open my $pagfh, ">:raw", $pagname
+ or die "Cannot recreate $pagname: $!";
+ print $pagfh $base64;
+ close $pagfh;
+ open my $dirfh, ">:raw", $dirname
+ or die "Cannot truncate $dirname: $!";
+ close $dirfh;
+
+ my %dbm;
+ my $sdbm = tie %dbm, 'SDBM_File', $dirname, O_RDWR, 0666, $pagname;
+
+ ok(tied %dbm, "$note: tied successfully");
+ my $value = $dbm{foo};
+ pass("$note: no crash fetching a named key");
+ my $tmp;
+ for my $key (sort keys %dbm) {
+ $tmp = $dbm{$key};
+ }
+ pass("$note: no crash iterating over keys");
+ is(0+$!, EINVAL, "$note: errno set");
+ ok($sdbm->error, "$note: error flag set");
+}
+
+
+done_testing();
+
+__DATA__
+132147
+CgD+g/4D/QP8A/sD2gPZA8wDywPIAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABm
+OmZxYlpYWEhaZ0ExbDFQY2R7U0hBfWhLVVdoQnVuZWx0R1NONHMwTi9MTU9wRzI3UT1jYmJhYQ==
+END
+132151
+AiD/A/4DAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA8wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA5AAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAGZo
+END
--
2.11.0
|
From @tonycoz0002-perl-132147-don-t-cache-invalid-pages.patchFrom 97f6048395f32f98e30a9c0acd6362447c11bb10 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 6 Nov 2018 14:23:48 +1100
Subject: (perl #132147) don't cache invalid pages
When sdbm loads its page buffer from disk, in most cases it validates
the page and doesn't continue processing if it fails validation.
Unfortunately, in a few places it still marked the buffer as loaded
from that page, and later calls would then use that cached page,
causing a variety of problems, including buffer read overflows.
sdbm_firstkey() didn't validate the loaded page at all, it now does.
All places that validate the loaded page now on a failed validation:
- invalidate the cached page (set pagbno to -1)
- set the I/O error flag on the database object
- set errno ($!) to EINVAL
The first ensures that later calls don't end up using an invalid cached
page.
The others allow the caller to check whether an error has occurred.
---
ext/SDBM_File/sdbm.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/ext/SDBM_File/sdbm.c b/ext/SDBM_File/sdbm.c
index 2099857fb8..bdb5f47cf5 100644
--- a/ext/SDBM_File/sdbm.c
+++ b/ext/SDBM_File/sdbm.c
@@ -398,6 +398,12 @@ sdbm_firstkey(DBM *db)
if (lseek(db->pagf, OFF_PAG(0), SEEK_SET) < 0
|| read(db->pagf, db->pagbuf, PBLKSIZ) < 0)
return ioerr(db), nullitem;
+ if (!chkpage(db->pagbuf)) {
+ errno = EINVAL;
+ ioerr(db);
+ db->pagbno = -1;
+ return nullitem;
+ }
db->pagbno = 0;
db->blkptr = 0;
db->keyptr = 0;
@@ -446,8 +452,12 @@ getpage(DBM *db, long int hash)
if (lseek(db->pagf, OFF_PAG(pagb), SEEK_SET) < 0
|| read(db->pagf, db->pagbuf, PBLKSIZ) < 0)
return 0;
- if (!chkpage(db->pagbuf))
- return 0;
+ if (!chkpage(db->pagbuf)) {
+ errno = EINVAL;
+ db->pagbno = -1;
+ ioerr(db);
+ return 0;
+ }
db->pagbno = pagb;
debug(("pag read: %d\n", pagb));
@@ -543,8 +553,12 @@ getnext(DBM *db)
db->pagbno = db->blkptr;
if (read(db->pagf, db->pagbuf, PBLKSIZ) <= 0)
break;
- if (!chkpage(db->pagbuf))
- break;
+ if (!chkpage(db->pagbuf)) {
+ errno = EINVAL;
+ db->pagbno = -1;
+ ioerr(db);
+ break;
+ }
}
return ioerr(db), nullitem;
--
2.11.0
|
From @tonycoz0003-perl-132147-add-extra-block-validation-checks.patchFrom bd09455ad3c039aca22f31758eb8ec2ff660203a Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 7 Nov 2018 11:16:10 +1100
Subject: (perl #132147) add extra block validation checks
and a few extra tests that fuzz testing found.
---
ext/SDBM_File/pair.c | 22 +++++++++++++++-
ext/SDBM_File/t/corrupt.t | 64 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/ext/SDBM_File/pair.c b/ext/SDBM_File/pair.c
index 2e4d8074e5..c12ad334e6 100644
--- a/ext/SDBM_File/pair.c
+++ b/ext/SDBM_File/pair.c
@@ -269,6 +269,20 @@ splpage(char *pag, char *New, long int sbit)
* reasonable, and all offsets in the index should be in order.
* this could be made more rigorous.
*/
+/*
+ Layout of a page is:
+ Top of block:
+ number of keys/values (short)
+ Array of (number of keys/values) offsets, alternating between key offsets
+ and value offsets (shorts)
+ End of block:
+ - value/key data, last key ends at end of block (bytes)
+
+ So:
+ N key0off val0off key1off val1off ... val1 key1 val0 key0
+
+ Be careful to note N is the number of offsets, *not* the number of keys.
+ */
int
chkpage(char *pag)
{
@@ -283,11 +297,17 @@ chkpage(char *pag)
off = PBLKSIZ;
for (ino++; n > 0; ino += 2) {
if (ino[0] > off || ino[1] > off ||
- ino[1] > ino[0])
+ ino[1] > ino[0] || ino[1] <= 0)
return 0;
off = ino[1];
n -= 2;
}
+ /* there must be an even number of offsets */
+ if (n != 0)
+ return 0;
+ /* check the key/value offsets don't overlap the key/value data */
+ if ((char *)ino > pag + off)
+ return 0;
}
return 1;
}
diff --git a/ext/SDBM_File/t/corrupt.t b/ext/SDBM_File/t/corrupt.t
index b4e41aab64..30c7b507a2 100644
--- a/ext/SDBM_File/t/corrupt.t
+++ b/ext/SDBM_File/t/corrupt.t
@@ -12,6 +12,7 @@ my ($pagfh, $pagname) = tempfile(UNLINK => 1);
close $dirfh;
close pagefh;
+# these might only fail under ASAN
while (my $testdata = do { local $/ = "END\n"; <DATA>; }) {
my ($note, $base64) = $testdata =~ /\A([^\n]+)\n(.*)/s
or die;
@@ -30,9 +31,8 @@ while (my $testdata = do { local $/ = "END\n"; <DATA>; }) {
ok(tied %dbm, "$note: tied successfully");
my $value = $dbm{foo};
pass("$note: no crash fetching a named key");
- my $tmp;
for my $key (sort keys %dbm) {
- $tmp = $dbm{$key};
+ my $tmp = $dbm{$key};
}
pass("$note: no crash iterating over keys");
is(0+$!, EINVAL, "$note: errno set");
@@ -84,3 +84,63 @@ AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAGZo
END
+fuzz failure 54
+EADwA78DsQNpA1YDIgMWA8ECswJqAmACPQImAuEBywGqtgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAjAAAAAAAAABoAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAsQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAPP9hwAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAATAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADBAAAAAABtAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAeHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHgs
+eHh4YWJjZGVmZ2hpamtsbW5vcHFyNDg2NHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHjkeHh4eHh4eGFiY2RlZmdoaWprbG1ub3BxcnMy
+MTU5eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eEYxMjM0NTY3NjI2eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4
+eHh4eHh4eGFiY2RlZmdoaWoyMzU4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4
+eHh4X3h4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eDEyMzQ1Njc4
+MzE3MXh4eHh4eHh4eHiYeHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHhh
+YmNkZWZnaGlqa2xtbm8zMDE2eHh4eLh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4
+eHiqeHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4MTIzNDU2Yzg5MDI3NjJ4eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eE14eHh4eHh4YWJjZGVmZ2hpamtsbW5vcA==
+END
+fuzz failure 181
+BAD/A8MDuAOduwAAAAAAAAAAAAAAAAAAAAAAAAAAUwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAJQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AABEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eDEyMzQ1NjcyOTczeHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4MQ==
+END
+fuzz failure 695
+EADoA+cD3wN/A3kDIAMFA+wC3wKeApkCgQJ3AmgCYgJdtwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+ADMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGoAAAAA
+AAAAAAAAAAAAAAAAAAAAAAAAAACTAAAAAAAAAAAAAAAAAAB4eHh4eGFiY2RlZnh4eHh4eHh4eHh4
+eHh4eGFiY2RlZjkyMzZ4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHgxMzUyMnh4eHh4eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4YWJjZGVm
+Z2hpNTUxOXh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHhhYmNkZWZnaGlqa2xtbm9wcXJzdHV2dzg1
+NTF4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eDEyMTE3NHh4eHh4eHh4eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4
+eHh4eHh4eHh4eHh4eHh4eHh4eHh4eDEyMzQ1NTMyeGFiY2RlZmdoaWprbG1ub3BxcnN0ODE1Mg==
+END
--
2.11.0
|
From @tonycozOn Tue, 06 Nov 2018 19:17:27 -0800, tonyc wrote:
The attached are the programs I used for fuzz testing. Some of the code in fuzz_sdbm.pl was also intended for checking I hadn't broken anything. I had both in the parent directory of my perl source tree and ran: ./perl -Ilib fuzz_sdbm.pl which would then generate a clean database, randomly scribble on it and see if they crashed when opened with fuzzrun.pl. For other formats I would have considered insertions and deletions, but I don't think that adds anything for the sdbm format. Tony [1] many of the possible changes fuzz_sdbm.pl makes don't damage the database at all, but many do |
From @tonycoz |
From @tonycoz |
From @tonycozOn Tue, 06 Nov 2018 19:17:27 -0800, tonyc wrote:
Applied as part of the 7d5be4b merge. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
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#132147 (status was 'resolved')
Searchable as RT132147$
The text was updated successfully, but these errors were encountered: