Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDBM Memory Safety Issues #16164

Closed
p5pRT opened this issue Sep 22, 2017 · 21 comments
Closed

SDBM Memory Safety Issues #16164

p5pRT opened this issue Sep 22, 2017 · 21 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 22, 2017

Migrated from rt.perl.org#132147 (status was 'resolved')

Searchable as RT132147$

@p5pRT
Copy link
Author

p5pRT commented Sep 22, 2017

From CYoung@tripwire.com

I 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​:
cyoung@​FuzzBox​:~$ base64 perl/fuzz/sdbm/demo.dbm.pag
CgD+g/4D/QP8A/sD2gPZA8wDywPIAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABm
OmZxYlpYWEhaZ0ExbDFQY2R7U0hBfWhLVVdoQnVuZWx0R1NONHMwTi9MTU9wRzI3UT1jYmJhYQ==

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)​:
#!/usr/bin/perl
#opendbm.plx

use warnings;
use strict;
use POSIX;
use SDBM_File;

my %dbm;
my $db_file="demo.dbm";

tie %dbm, 'SDBM_File', $db_file, O_RDWR, 0;

my $value=$dbm{'foo'};

foreach (sort keys(%dbm)) {
  print "$_ => $dbm{$_}\n";
}

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​:
cyoung@​FuzzBox​:~/perl/fuzz/sdbm$ perl test_sdbm.pl

==16987==ERROR​: AddressSanitizer​: heap-buffer-overflow on address 0x622000010546 at pc 0x0000004becc7 bp 0x7ffeeb931650 sp 0x7ffeeb930e00
READ of size 32770 at 0x622000010546 thread T0
  #0 0x4becc6 in __asan_memmove (/usr/local/bin/perl+0x4becc6)
  #1 0x852463 in Perl_sv_setpvn /home/cyoung/perl/perl5-5.26.1-RC1/sv.c​:5004​:5
  #2 0x7efd56e7792e in XS_SDBM_File_FIRSTKEY /home/cyoung/perl/perl5-5.26.1-RC1/ext/SDBM_File/SDBM_File.c​:479​:6
  #3 0x8308c1 in Perl_pp_entersub /home/cyoung/perl/perl5-5.26.1-RC1/pp_hot.c​:4231​:2
  #4 0x804956 in Perl_runops_standard /home/cyoung/perl/perl5-5.26.1-RC1/run.c​:41​:26
  #5 0x5b3816 in Perl_call_sv /home/cyoung/perl/perl5-5.26.1-RC1/perl.c​:2831​:2
  #6 0x7a2aa5 in Perl_magic_methcall /home/cyoung/perl/perl5-5.26.1-RC1/mg.c​:1839​:6
  #7 0x7a3fad in Perl_magic_nextpack /home/cyoung/perl/perl5-5.26.1-RC1/mg.c​:1975​:4
  #8 0x7e98b4 in Perl_hv_iternext_flags /home/cyoung/perl/perl5-5.26.1-RC1/hv.c​:2627​:13
  #9 0x9dda84 in Perl_do_kv /home/cyoung/perl/perl5-5.26.1-RC1/doop.c​:1325​:21
  #10 0x804956 in Perl_runops_standard /home/cyoung/perl/perl5-5.26.1-RC1/run.c​:41​:26
  #11 0x5b1d86 in S_run_body /home/cyoung/perl/perl5-5.26.1-RC1/perl.c​:2524​:2
  #12 0x5b1d86 in perl_run /home/cyoung/perl/perl5-5.26.1-RC1/perl.c​:2447
  #13 0x5060ba in main /home/cyoung/perl/perl5-5.26.1-RC1/perlmain.c​:123​:9
  #14 0x7efd5a00d82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c​:291
  #15 0x4347e8 in _start (/usr/local/bin/perl+0x4347e8)

AddressSanitizer can not describe address in more detail (wild memory access suspected).
SUMMARY​: AddressSanitizer​: heap-buffer-overflow (/usr/local/bin/perl+0x4becc6) in __asan_memmove
Shadow bytes around the buggy address​:
  0x0c447fffa050​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa060​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa070​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa080​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa090​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c447fffa0a0​: fa fa fa fa fa fa fa fa[fa]fa fa fa fa fa fa fa
  0x0c447fffa0b0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa0c0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa0d0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa0e0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c447fffa0f0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes)​:
  Addressable​: 00
  Partially addressable​: 01 02 03 04 05 06 07
  Heap left redzone​: fa
  Heap right redzone​: fb
  Freed heap region​: fd
  Stack left redzone​: f1
  Stack mid redzone​: f2
  Stack right redzone​: f3
  Stack partial redzone​: f4
  Stack after return​: f5
  Stack use after scope​: f8
  Global redzone​: f9
  Global init order​: f6
  Poisoned by user​: f7
  Container overflow​: fc
  Array cookie​: ac
  Intra object redzone​: bb
  ASan internal​: fe
  Left alloca redzone​: ca
  Right alloca redzone​: cb
==16987==ABORTING

It is also important to mention that this test case also triggers a similar condition within Apache APR’s sdbm code which I have been advised was forked from the SDBM_File code at some point. The APR team already has a patch prepared to add bounds checks. It may be worth coordinating disclosure with ASF to minimize risk to end users.
Thanks,
Craig
________________
Craig Young | Security Researcher
Direct​: 404-492-9657
TRIPWIRE | CONFIDENCE​: SECURED
www.tripwire.com<http​://www.tripwire.com/>

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2017

From @tonycoz

On Fri, 22 Sep 2017 16​:16​:03 -0700, CYoung@​tripwire.com wrote​:

I have identified a test case which triggers a memory safety issue (as
detected by ASAN) within the SDBM_File module.

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

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2017

From CYoung@tripwire.com

Yes, 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,
Craig

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​:
  > I have identified a test case which triggers a memory safety issue (as
  > detected by ASAN) within the SDBM_File module.
 
  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
 

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2017

From @iabyn

On Mon, Sep 25, 2017 at 02​:25​:28PM +0000, Craig Young wrote​:

Yes, 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.

I think it would take considerable effort to audit and fix the sdbm code
against all conceivable malicious dbm files.

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.

So instead, I propose that we add a warning to the pod file (and maybe
[GNO]DBM_File too?) then close this ticket and the related RT #132151.

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
  portability or security. A maliciously crafted file might cause perl to
  crash or even expose a security vulnerability.

--
"Emacs isn't a bad OS once you get used to it.
It just lacks a decent editor."

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2017

From CYoung@tripwire.com

This sounds good.
Thanks,
Craig
________________________________________
From​: Dave Mitchell via RT [perl5-security-report-followup@​perl.org]
Sent​: Wednesday, November 29, 2017 6​:18 AM
To​: Craig Young
Subject​: Re​: [perl #132147] SDBM Memory Safety Issues

On Mon, Sep 25, 2017 at 02​:25​:28PM +0000, Craig Young wrote​:

Yes, 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.

I think it would take considerable effort to audit and fix the sdbm code
against all conceivable malicious dbm files.

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.

So instead, I propose that we add a warning to the pod file (and maybe
[GNO]DBM_File too?) then close this ticket and the related RT #132151.

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
  portability or security. A maliciously crafted file might cause perl to
  crash or even expose a security vulnerability.

--
"Emacs isn't a bad OS once you get used to it.
It just lacks a decent editor."

@p5pRT
Copy link
Author

p5pRT commented Jan 24, 2018

From @tonycoz

On Wed, 29 Nov 2017 03​:18​:41 -0800, davem wrote​:

On Mon, Sep 25, 2017 at 02​:25​:28PM +0000, Craig Young wrote​:

Yes, 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.

I think it would take considerable effort to audit and fix the sdbm code
against all conceivable malicious dbm files.

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.

So instead, I propose that we add a warning to the pod file (and maybe
[GNO]DBM_File too?) then close this ticket and the related RT #132151.

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
portability or security\. A maliciously crafted file might cause perl to
crash or even expose a security vulnerability\.

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

@p5pRT
Copy link
Author

p5pRT commented Jan 24, 2018

From @tonycoz

sdbm_file.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Apr 9, 2018

From @iabyn

On Tue, Jan 23, 2018 at 08​:21​:54PM -0800, Tony Cook via RT wrote​:

On Wed, 29 Nov 2017 03​:18​:41 -0800, davem wrote​:

On Mon, Sep 25, 2017 at 02​:25​:28PM +0000, Craig Young wrote​:

Yes, 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.

I think it would take considerable effort to audit and fix the sdbm code
against all conceivable malicious dbm files.

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.

So instead, I propose that we add a warning to the pod file (and maybe
[GNO]DBM_File too?) then close this ticket and the related RT #132151.

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
portability or security\. A maliciously crafted file might cause perl to
crash or even expose a security vulnerability\.

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.

Looks fine to me.

If anyone has a link to GDBM being safe with untrusted files, I'd
appreciate a link, I couldn't find anything with Google.

No idea.

--
You're only as old as you look.

@p5pRT
Copy link
Author

p5pRT commented Nov 4, 2018

From @tonycoz

On Tue, 23 Jan 2018 20​:21​:54 -0800, tonyc 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.

Applied as e459aaf and 328d907.

Tony

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

On Sun, 04 Nov 2018 15​:38​:33 -0800, tonyc wrote​:

On Tue, 23 Jan 2018 20​:21​:54 -0800, tonyc 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.

Applied as e459aaf and
328d907.

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

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

0001-perl-132147-add-tests-for-corrupt-files-from-tickets.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

0002-perl-132147-don-t-cache-invalid-pages.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

0003-perl-132147-add-extra-block-validation-checks.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

On Tue, 06 Nov 2018 19​:17​:27 -0800, tonyc wrote​:

found by fuzz testing

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

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

fuzz_sdbm.pl

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2018

From @tonycoz

fuzzrun.pl

@p5pRT
Copy link
Author

p5pRT commented Nov 19, 2018

From @tonycoz

On Tue, 06 Nov 2018 19​:17​:27 -0800, tonyc wrote​:

On Sun, 04 Nov 2018 15​:38​:33 -0800, tonyc wrote​:

On Tue, 23 Jan 2018 20​:21​:54 -0800, tonyc 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.

Applied as e459aaf and
328d907.

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).

Applied as part of the 7d5be4b merge.

Tony

@p5pRT
Copy link
Author

p5pRT commented Nov 19, 2018

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented May 22, 2019

From @khwilliamson

Thank 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
resolved.

Perl 5.30.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented May 22, 2019

@khwilliamson - Status changed from 'pending release' to 'resolved'

@p5pRT p5pRT closed this as completed May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant