Skip Menu |
Report information
Id: 132147
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: CYoung [at] tripwire.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: (no value)
Type: (no value)
Perl Version: (no value)
Fixed In: (no value)



Date: Fri, 22 Sep 2017 20:42:38 +0000
To: "perl5-security-report [...] perl.org" <perl5-security-report [...] perl.org>
Subject: SDBM Memory Safety Issues
From: Craig Young <CYoung [...] tripwire.com>
Download (untitled) / with headers
text/html 18.3k

Message body is not shown because it is too large.

RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 550b
On Fri, 22 Sep 2017 16:16:03 -0700, CYoung@tripwire.com wrote: Show quoted text
> 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
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
Date: Mon, 25 Sep 2017 14:25:28 +0000
Subject: Re: [perl #132147] SDBM Memory Safety Issues
From: Craig Young <CYoung [...] tripwire.com>
Download (untitled) / with headers
text/plain 1.5k
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: Show quoted text
> 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
Date: Wed, 29 Nov 2017 11:18:19 +0000
To: Craig Young <CYoung [...] tripwire.com>
CC: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #132147] SDBM Memory Safety Issues
On Mon, Sep 25, 2017 at 02:25:28PM +0000, Craig Young wrote: Show quoted text
> 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. Show quoted text
> 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."
Date: Wed, 29 Nov 2017 13:07:13 +0000
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
Subject: RE: [perl #132147] SDBM Memory Safety Issues
From: Craig Young <cyoung [...] tripwire.com>
Download (untitled) / with headers
text/plain 1.3k
This sounds good. Thanks, Craig Show quoted text
________________________________________ 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."
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Wed, 29 Nov 2017 03:18:41 -0800, davem wrote: Show quoted text
> 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
Subject: sdbm_file.patch
Download sdbm_file.patch
text/plain 5.7k
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
To: Tony Cook via RT <perl5-security-report-followup [...] perl.org>
Date: Mon, 9 Apr 2018 17:11:13 +0100
CC: perl5-security-report [...] perl.org
Subject: Re: [perl #132147] SDBM Memory Safety Issues
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 1.5k
On Tue, Jan 23, 2018 at 08:21:54PM -0800, Tony Cook via RT wrote: Show quoted text
> 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. Show quoted text
> 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.
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 319b
On Tue, 23 Jan 2018 20:21:54 -0800, tonyc wrote: Show quoted text
> 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 e459aaffe40291395017cc002fc6d261e7cae0ae and 328d9079796a9f9f8dfb4813f36e50e8a77a0748. Tony
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 630b
On Sun, 04 Nov 2018 15:38:33 -0800, tonyc wrote: Show quoted text
> 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 e459aaffe40291395017cc002fc6d261e7cae0ae and > 328d9079796a9f9f8dfb4813f36e50e8a77a0748.
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
Subject: 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
Subject: 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
Subject: 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
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 670b
On Tue, 06 Nov 2018 19:17:27 -0800, tonyc wrote: Show quoted text
> 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
Subject: fuzz_sdbm.pl
Download fuzz_sdbm.pl
text/x-perl 2.2k
#!perl use strict; use SDBM_File; use File::Copy; use File::Temp qw(tempfile); use Fcntl; my ($dirfh, $dirname) = tempfile(UNLINK => 1); my ($pagfh, $pagname) = tempfile(UNLINK => 1); close $dirfh; close $pagfh; my @keys = qw(1234567890a abcdefghijklmnopqrstuvwxyz); -d "fuzzfails" or mkdir "fuzzfails" or die "mkdir fuzzfails: $!"; my $count = 0; while (1) { my %dbm; unlink $dirname; unlink $pagname; tie %dbm, "SDBM_File", $dirname, O_CREAT | O_RDWR, 0666, $pagname or die "Couldn't create: $!"; my $changes = ''; my $keycount = int(rand 10) + int(exp(rand 7.6)); my $total_keys = 2; # key count entry my %keys; for my $keynum (1 .. $keycount) { my $key = $keys[rand @keys]; substr($key, int(1 + rand(length($key)-1))) = ''; if (rand(1) < 0.2) { $key .= int rand(10000); } my $val = "x" x (1+int rand(100)); $dbm{$key} = $val; $keys{$key} = $val; $changes .= "[$key] = '$val'\n"; $total_keys += 4 + length($key) + length($val); } untie %dbm; my $pagdata = slurp($pagname); my $dirdata = slurp($dirname); tie %dbm, "SDBM_File", $dirname, O_RDWR, 0666, $pagname or die "Couldn't reopen: $!"; for my $key (keys %keys) { unless ($dbm{$key} eq $keys{$key}) { die "Couldn't find $key"; } } untie %dbm; my $change_count = int(1+rand(20)); for my $i (1 .. $change_count) { my $pos = int rand(length($pagdata)); my $byte = int rand(256); $changes .= " $pos => $byte\n"; substr($pagdata, $pos, 1, chr($byte)); } splat($pagname, $pagdata); ++$count; print ">>> $count ($keycount keys, $total_keys size, $change_count changes)\n"; print ">>> *** on block boundary\n" if $total_keys % 1024 == 0; my $run = "$^X -Ilib ../fuzzrun.pl $dirname $pagname"; if (system $run) { print "<<< fail $? '$run'\n"; splat("fuzzfails/$count.pag", $pagdata); splat("fuzzfails/$count.dir", $dirdata); splat("fuzzfails/$count.changes", $changes); } else { print "<<< good\n"; } #exit; } sub slurp { my ($name) = @_; open my $fh, "<:raw", $name or die $!; return do { local $/; <$fh> }; } sub splat { my ($name, $data) = @_; open my $fh, ">:raw", $name or die $!; print $fh $data; close $fh or die $!; }
Subject: fuzzrun.pl
Download fuzzrun.pl
text/x-perl 267b
#!perl use strict; use SDBM_File; use Fcntl; my ($dirname, $pagname) = @ARGV; $pagname or die; tie my %dbm, "SDBM_File", $dirname, O_RDWR, 0666, $pagname or exit; my $val = $dbm{abcdef}; for my $key (keys %dbm) { #print "key $key\n"; my $val = $dbm{$key}; }
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 780b
On Tue, 06 Nov 2018 19:17:27 -0800, tonyc wrote: Show quoted text
> 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 e459aaffe40291395017cc002fc6d261e7cae0ae and > > 328d9079796a9f9f8dfb4813f36e50e8a77a0748.
> > 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 7d5be4b67af37e3a46105aeafc0c7871322bdc7d merge. Tony
Download (untitled) / with headers
text/plain 313b
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.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org