From be7ce5ac9fbdbc1cdb2d10edc5c58f41ff38bb61 Mon Sep 17 00:00:00 2001 From: Guillem Jover Date: Thu, 19 Mar 2015 22:51:46 +0100 Subject: [PATCH] Dpkg::Control::HashCore: Fix OpenPGP Armor Header Line parsing We should only accept [\r\t ] as trailing whitespace, although RFC4880 does not clarify what whitespace really maps to, we should really match the GnuPG implementation anyway, as that is what we use to verify the signatures. Fixes: CVE-2015-0840 Reported-by: Jann Horn --- scripts/Dpkg/Control/Hash.pm | 28 ++++++++++++---------- scripts/Makefile.am | 1 + scripts/t/700_Dpkg_Control.t | 5 +++- .../t/700_Dpkg_Control/bogus-armor-formfeed.dsc | 19 +++++++++++++++ 4 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 scripts/t/700_Dpkg_Control/bogus-armor-formfeed.dsc diff --git a/scripts/Dpkg/Control/Hash.pm b/scripts/Dpkg/Control/Hash.pm index b142876..a21b75d 100644 --- a/scripts/Dpkg/Control/Hash.pm +++ b/scripts/Dpkg/Control/Hash.pm @@ -175,19 +175,21 @@ sub parse { my $pgp_signed = 0; while (<$fh>) { - s/\s*\n$//; - next if (m/^$/ and $paraborder); + chomp; + next if m/^\s*$/ and $paraborder; next if (m/^#/); $paraborder = 0; if (m/^(\S+?)\s*:\s*(.*)$/) { $parabody = 1; - if (exists $self->{$1}) { + my ($name, $value) = ($1, $2); + if (exists $self->{$name}) { unless ($$self->{'allow_duplicate'}) { - syntaxerr($desc, sprintf(_g("duplicate field %s found"), $1)); + syntaxerr($desc, sprintf(_g("duplicate field %s found"), $name)); } } - $self->{$1} = $2; - $cf = $1; + $value =~ s/\s*$//; + $self->{$name} = $value; + $cf = $name; } elsif (m/^\s(\s*\S.*)$/) { my $line = $1; unless (defined($cf)) { @@ -196,8 +198,9 @@ sub parse { if ($line =~ /^\.+$/) { $line = substr $line, 1; } + $line =~ s/\s*$//; $self->{$cf} .= "\n$line"; - } elsif (m/^-----BEGIN PGP SIGNED MESSAGE-----$/) { + } elsif (m/^-----BEGIN PGP SIGNED MESSAGE-----[\r\t ]*$/) { $expect_pgp_sig = 1; if ($$self->{'allow_pgp'} and not $parabody) { # Skip PGP headers @@ -207,22 +210,23 @@ sub parse { } else { syntaxerr($desc, _g("PGP signature not allowed here")); } - } elsif (m/^$/ || ($expect_pgp_sig && m/^-----BEGIN PGP SIGNATURE-----$/)) { + } elsif (m/^\s*$/ || + ($expect_pgp_sig && m/^-----BEGIN PGP SIGNATURE-----[\r\t ]*$/)) { if ($expect_pgp_sig) { # Skip empty lines $_ = <$fh> while defined($_) && $_ =~ /^\s*$/; length($_) || syntaxerr($desc, _g("expected PGP signature, found EOF " . "after blank line")); - s/\s*\n$//; - unless (m/^-----BEGIN PGP SIGNATURE-----$/) { + chomp; + unless (m/^-----BEGIN PGP SIGNATURE-----[\r\t ]*$/) { syntaxerr($desc, sprintf(_g("expected PGP signature, " . "found something else \`%s'"), $_)); } # Skip PGP signature while (<$fh>) { - s/\s*\n$//; - last if m/^-----END PGP SIGNATURE-----$/; + chomp; + last if m/^-----END PGP SIGNATURE-----[\r\t ]*$/; } unless (defined($_)) { syntaxerr($desc, _g("unfinished PGP signature")); diff --git a/scripts/Makefile.am b/scripts/Makefile.am index fd63991..0289437 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -231,6 +231,7 @@ test_data = \ t/700_Dpkg_Control/control-1 \ t/700_Dpkg_Control/bogus-unsigned.dsc \ t/700_Dpkg_Control/bogus-armor-double.dsc \ + t/700_Dpkg_Control/bogus-armor-formfeed.dsc \ t/700_Dpkg_Control/bogus-armor-no-sig.dsc \ t/700_Dpkg_Control/bogus-armor-trail.dsc \ t/700_Dpkg_Control/bogus-armor-inline.dsc \ diff --git a/scripts/t/700_Dpkg_Control.t b/scripts/t/700_Dpkg_Control.t index 074e084..2331b10 100644 --- a/scripts/t/700_Dpkg_Control.t +++ b/scripts/t/700_Dpkg_Control.t @@ -13,7 +13,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -use Test::More tests => 22; +use Test::More tests => 23; use strict; use warnings; @@ -118,6 +118,9 @@ is($dsc, undef, 'Signed .dsc w/ bogus OpenPGP armor trailer'); $dsc = parse_dsc("$datadir/bogus-armor-inline.dsc"); is($dsc, undef, 'Signed .dsc w/ bogus OpenPGP inline armor'); +$dsc = parse_dsc("$datadir/bogus-armor-formfeed.dsc"); +is($dsc, undef, 'Signed .dsc w/ bogus OpenPGP armor line'); + $dsc = parse_dsc("$datadir/bogus-armor-double.dsc"); ok(defined $dsc, 'Signed .dsc w/ two OpenPGP armor signatures'); is($dsc->{Source}, 'pass', 'Signed spaced .dsc package name'); diff --git a/scripts/t/700_Dpkg_Control/bogus-armor-formfeed.dsc b/scripts/t/700_Dpkg_Control/bogus-armor-formfeed.dsc new file mode 100644 index 0000000..70aab18 --- /dev/null +++ b/scripts/t/700_Dpkg_Control/bogus-armor-formfeed.dsc @@ -0,0 +1,19 @@ +-----BEGIN PGP SIGNED MESSAGE----- + +Source: fail + +-----BEGIN PGP SIGNATURE----- +Version: vim v7.3.547 (GNU/Linux) + +Fake signature here. +-----END PGP SIGNATURE----- +-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA1 + +Source: pass + +-----BEGIN PGP SIGNATURE +Version: GnuPG v1.4.12 (GNU/Linux) + +Valid signature here. +-----END PGP SIGNATURE----- -- 2.7.4