Revision 624f5ab8720b3371367327a822c267699c1823b8 authored by Eric Biggers on 07 November 2017, 22:29:02 UTC, committed by James Morris on 08 November 2017, 13:38:21 UTC
syzkaller reported a NULL pointer dereference in asn1_ber_decoder().  It
can be reproduced by the following command, assuming
CONFIG_PKCS7_TEST_KEY=y:

        keyctl add pkcs7_test desc '' @s

The bug is that if the data buffer is empty, an integer underflow occurs
in the following check:

        if (unlikely(dp >= datalen - 1))
                goto data_overrun_error;

This results in the NULL data pointer being dereferenced.

Fix it by checking for 'datalen - dp < 2' instead.

Also fix the similar check for 'dp >= datalen - n' later in the same
function.  That one possibly could result in a buffer overread.

The NULL pointer dereference was reproducible using the "pkcs7_test" key
type but not the "asymmetric" key type because the "asymmetric" key type
checks for a 0-length payload before calling into the ASN.1 decoder but
the "pkcs7_test" key type does not.

The bug report was:

    BUG: unable to handle kernel NULL pointer dereference at           (null)
    IP: asn1_ber_decoder+0x17f/0xe60 lib/asn1_decoder.c:233
    PGD 7b708067 P4D 7b708067 PUD 7b6ee067 PMD 0
    Oops: 0000 [#1] SMP
    Modules linked in:
    CPU: 0 PID: 522 Comm: syz-executor1 Not tainted 4.14.0-rc8 #7
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.3-20171021_125229-anatol 04/01/2014
    task: ffff9b6b3798c040 task.stack: ffff9b6b37970000
    RIP: 0010:asn1_ber_decoder+0x17f/0xe60 lib/asn1_decoder.c:233
    RSP: 0018:ffff9b6b37973c78 EFLAGS: 00010216
    RAX: 0000000000000000 RBX: 0000000000000000 RCX: 000000000000021c
    RDX: ffffffff814a04ed RSI: ffffb1524066e000 RDI: ffffffff910759e0
    RBP: ffff9b6b37973d60 R08: 0000000000000001 R09: ffff9b6b3caa4180
    R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000002
    R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
    FS:  00007f10ed1f2700(0000) GS:ffff9b6b3ea00000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000000 CR3: 000000007b6f3000 CR4: 00000000000006f0
    Call Trace:
     pkcs7_parse_message+0xee/0x240 crypto/asymmetric_keys/pkcs7_parser.c:139
     verify_pkcs7_signature+0x33/0x180 certs/system_keyring.c:216
     pkcs7_preparse+0x41/0x70 crypto/asymmetric_keys/pkcs7_key_type.c:63
     key_create_or_update+0x180/0x530 security/keys/key.c:855
     SYSC_add_key security/keys/keyctl.c:122 [inline]
     SyS_add_key+0xbf/0x250 security/keys/keyctl.c:62
     entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x4585c9
    RSP: 002b:00007f10ed1f1bd8 EFLAGS: 00000216 ORIG_RAX: 00000000000000f8
    RAX: ffffffffffffffda RBX: 00007f10ed1f2700 RCX: 00000000004585c9
    RDX: 0000000020000000 RSI: 0000000020008ffb RDI: 0000000020008000
    RBP: 0000000000000000 R08: ffffffffffffffff R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000216 R12: 00007fff1b2260ae
    R13: 00007fff1b2260af R14: 00007f10ed1f2700 R15: 0000000000000000
    Code: dd ca ff 48 8b 45 88 48 83 e8 01 4c 39 f0 0f 86 a8 07 00 00 e8 53 dd ca ff 49 8d 46 01 48 89 85 58 ff ff ff 48 8b 85 60 ff ff ff <42> 0f b6 0c 30 89 c8 88 8d 75 ff ff ff 83 e0 1f 89 8d 28 ff ff
    RIP: asn1_ber_decoder+0x17f/0xe60 lib/asn1_decoder.c:233 RSP: ffff9b6b37973c78
    CR2: 0000000000000000

Fixes: 42d5ec27f873 ("X.509: Add an ASN.1 decoder")
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: <stable@vger.kernel.org> # v3.7+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
1 parent fbc3edf
Raw File
headerdep.pl
#! /usr/bin/env perl
# SPDX-License-Identifier: GPL-2.0
#
# Detect cycles in the header file dependency graph
# Vegard Nossum <vegardno@ifi.uio.no>
#

use strict;
use warnings;

use Getopt::Long;

my $opt_all;
my @opt_include;
my $opt_graph;

&Getopt::Long::Configure(qw(bundling pass_through));
&GetOptions(
	help	=> \&help,
	version	=> \&version,

	all	=> \$opt_all,
	"I=s"	=> \@opt_include,
	graph	=> \$opt_graph,
);

push @opt_include, 'include';
my %deps = ();
my %linenos = ();

my @headers = grep { strip($_) } @ARGV;

parse_all(@headers);

if($opt_graph) {
	graph();
} else {
	detect_cycles(@headers);
}


sub help {
	print "Usage: $0 [options] file...\n";
	print "\n";
	print "Options:\n";
	print "  --all\n";
	print "  --graph\n";
	print "\n";
	print "  -I includedir\n";
	print "\n";
	print "To make nice graphs, try:\n";
	print "  $0 --graph include/linux/kernel.h | dot -Tpng -o graph.png\n";
	exit;
}

sub version {
	print "headerdep version 2\n";
	exit;
}

# Get a file name that is relative to our include paths
sub strip {
	my $filename = shift;

	for my $i (@opt_include) {
		my $stripped = $filename;
		$stripped =~ s/^$i\///;

		return $stripped if $stripped ne $filename;
	}

	return $filename;
}

# Search for the file name in the list of include paths
sub search {
	my $filename = shift;
	return $filename if -f $filename;

	for my $i (@opt_include) {
		my $path = "$i/$filename";
		return $path if -f $path;
	}
	return;
}

sub parse_all {
	# Parse all the headers.
	my @queue = @_;
	while(@queue) {
		my $header = pop @queue;
		next if exists $deps{$header};

		$deps{$header} = [] unless exists $deps{$header};

		my $path = search($header);
		next unless $path;

		open(my $file, '<', $path) or die($!);
		chomp(my @lines = <$file>);
		close($file);

		for my $i (0 .. $#lines) {
			my $line = $lines[$i];
			if(my($dep) = ($line =~ m/^#\s*include\s*<(.*?)>/)) {
				push @queue, $dep;
				push @{$deps{$header}}, [$i + 1, $dep];
			}
		}
	}
}

sub print_cycle {
	# $cycle[n] includes $cycle[n + 1];
	# $cycle[-1] will be the culprit
	my $cycle = shift;

	# Adjust the line numbers
	for my $i (0 .. $#$cycle - 1) {
		$cycle->[$i]->[0] = $cycle->[$i + 1]->[0];
	}
	$cycle->[-1]->[0] = 0;

	my $first = shift @$cycle;
	my $last = pop @$cycle;

	my $msg = "In file included";
	printf "%s from %s,\n", $msg, $last->[1] if defined $last;

	for my $header (reverse @$cycle) {
		printf "%s from %s:%d%s\n",
			" " x length $msg,
			$header->[1], $header->[0],
			$header->[1] eq $last->[1] ? ' <-- here' : '';
	}

	printf "%s:%d: warning: recursive header inclusion\n",
		$first->[1], $first->[0];
}

# Find and print the smallest cycle starting in the specified node.
sub detect_cycles {
	my @queue = map { [[0, $_]] } @_;
	while(@queue) {
		my $top = pop @queue;
		my $name = $top->[-1]->[1];

		for my $dep (@{$deps{$name}}) {
			my $chain = [@$top, [$dep->[0], $dep->[1]]];

			# If the dep already exists in the chain, we have a
			# cycle...
			if(grep { $_->[1] eq $dep->[1] } @$top) {
				print_cycle($chain);
				next if $opt_all;
				return;
			}

			push @queue, $chain;
		}
	}
}

sub mangle {
	$_ = shift;
	s/\//__/g;
	s/\./_/g;
	s/-/_/g;
	$_;
}

# Output dependency graph in GraphViz language.
sub graph {
	print "digraph {\n";

	print "\t/* vertices */\n";
	for my $header (keys %deps) {
		printf "\t%s [label=\"%s\"];\n",
			mangle($header), $header;
	}

	print "\n";

	print "\t/* edges */\n";
	for my $header (keys %deps) {
		for my $dep (@{$deps{$header}}) {
			printf "\t%s -> %s;\n",
				mangle($header), mangle($dep->[1]);
		}
	}

	print "}\n";
}
back to top