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
cleanfile
#!/usr/bin/env perl
# SPDX-License-Identifier: GPL-2.0
#
# Clean a text file -- or directory of text files -- of stealth whitespace.
# WARNING: this can be a highly destructive operation.  Use with caution.
#

use warnings;
use bytes;
use File::Basename;

# Default options
$max_width = 79;

# Clean up space-tab sequences, either by removing spaces or
# replacing them with tabs.
sub clean_space_tabs($)
{
    no bytes;			# Tab alignment depends on characters

    my($li) = @_;
    my($lo) = '';
    my $pos = 0;
    my $nsp = 0;
    my($i, $c);

    for ($i = 0; $i < length($li); $i++) {
	$c = substr($li, $i, 1);
	if ($c eq "\t") {
	    my $npos = ($pos+$nsp+8) & ~7;
	    my $ntab = ($npos >> 3) - ($pos >> 3);
	    $lo .= "\t" x $ntab;
	    $pos = $npos;
	    $nsp = 0;
	} elsif ($c eq "\n" || $c eq "\r") {
	    $lo .= " " x $nsp;
	    $pos += $nsp;
	    $nsp = 0;
	    $lo .= $c;
	    $pos = 0;
	} elsif ($c eq " ") {
	    $nsp++;
	} else {
	    $lo .= " " x $nsp;
	    $pos += $nsp;
	    $nsp = 0;
	    $lo .= $c;
	    $pos++;
	}
    }
    $lo .= " " x $nsp;
    return $lo;
}

# Compute the visual width of a string
sub strwidth($) {
    no bytes;			# Tab alignment depends on characters

    my($li) = @_;
    my($c, $i);
    my $pos = 0;
    my $mlen = 0;

    for ($i = 0; $i < length($li); $i++) {
	$c = substr($li,$i,1);
	if ($c eq "\t") {
	    $pos = ($pos+8) & ~7;
	} elsif ($c eq "\n") {
	    $mlen = $pos if ($pos > $mlen);
	    $pos = 0;
	} else {
	    $pos++;
	}
    }

    $mlen = $pos if ($pos > $mlen);
    return $mlen;
}

$name = basename($0);

@files = ();

while (defined($a = shift(@ARGV))) {
    if ($a =~ /^-/) {
	if ($a eq '-width' || $a eq '-w') {
	    $max_width = shift(@ARGV)+0;
	} else {
	    print STDERR "Usage: $name [-width #] files...\n";
	    exit 1;
	}
    } else {
	push(@files, $a);
    }
}

foreach $f ( @files ) {
    print STDERR "$name: $f\n";

    if (! -f $f) {
	print STDERR "$f: not a file\n";
	next;
    }

    if (!open(FILE, '+<', $f)) {
	print STDERR "$name: Cannot open file: $f: $!\n";
	next;
    }

    binmode FILE;

    # First, verify that it is not a binary file; consider any file
    # with a zero byte to be a binary file.  Is there any better, or
    # additional, heuristic that should be applied?
    $is_binary = 0;

    while (read(FILE, $data, 65536) > 0) {
	if ($data =~ /\0/) {
	    $is_binary = 1;
	    last;
	}
    }

    if ($is_binary) {
	print STDERR "$name: $f: binary file\n";
	next;
    }

    seek(FILE, 0, 0);

    $in_bytes = 0;
    $out_bytes = 0;
    $blank_bytes = 0;

    @blanks = ();
    @lines  = ();
    $lineno = 0;

    while ( defined($line = <FILE>) ) {
	$lineno++;
	$in_bytes += length($line);
	$line =~ s/[ \t\r]*$//;		# Remove trailing spaces
	$line = clean_space_tabs($line);

	if ( $line eq "\n" ) {
	    push(@blanks, $line);
	    $blank_bytes += length($line);
	} else {
	    push(@lines, @blanks);
	    $out_bytes += $blank_bytes;
	    push(@lines, $line);
	    $out_bytes += length($line);
	    @blanks = ();
	    $blank_bytes = 0;
	}

	$l_width = strwidth($line);
	if ($max_width && $l_width > $max_width) {
	    print STDERR
		"$f:$lineno: line exceeds $max_width characters ($l_width)\n";
	}
    }

    # Any blanks at the end of the file are discarded

    if ($in_bytes != $out_bytes) {
	# Only write to the file if changed
	seek(FILE, 0, 0);
	print FILE @lines;

	if ( !defined($where = tell(FILE)) ||
	     !truncate(FILE, $where) ) {
	    die "$name: Failed to truncate modified file: $f: $!\n";
	}
    }

    close(FILE);
}
back to top