From patchwork Mon Nov 6 22:21:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 10044579 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 277F0602BF for ; Mon, 6 Nov 2017 22:22:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 186D029E5C for ; Mon, 6 Nov 2017 22:22:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0D0F129E6D; Mon, 6 Nov 2017 22:22:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 56C8329E5E for ; Mon, 6 Nov 2017 22:22:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751516AbdKFWV7 (ORCPT ); Mon, 6 Nov 2017 17:21:59 -0500 Received: from mail-io0-f177.google.com ([209.85.223.177]:56472 "EHLO mail-io0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750728AbdKFWV5 (ORCPT ); Mon, 6 Nov 2017 17:21:57 -0500 Received: by mail-io0-f177.google.com with SMTP id m81so33796ioi.13; Mon, 06 Nov 2017 14:21:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=3PhvBnphXvS+8kjRFJ6z2JqGeGe5F6bivcTenkrBAu0=; b=pD5Cx92L1XJnTGi1pU87fQDzVWG5Hbkpq3v9pqIBjPtmcj1mhIrghu9hcVS9Per98I pbWoiIdOnVWwHpkSee/PqfYwelr6yTSYZfnJihIK9DEkCdApHfxbd9lyuu1j9xRE3dVv UOnmjl7Rxn5RDcnoUHsg0oaRGjp4ddOewXCI0DnTFCCXiXpxrTIs3VKjo4iduBoyHmrn 9f5gskDg2KXK3X3z2Z12nFYV+7tkq8dX/XT4RVfKMWgEVtwuGmA+mld3xWsJSU/cQqdx NL5ZLnfY5Yq+dUHI7dIRWdFVEe20akmdLdJU6e+Kq2eEBLNT2ZIjcDdbG8lpLmedQISW rAKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3PhvBnphXvS+8kjRFJ6z2JqGeGe5F6bivcTenkrBAu0=; b=F2kbZHNNHHMvaiUHUdt26+Si8dVzK/ihHdGkOyErveiNDysWClGL/QREiBRciLnl7S /hXrYH0KfL4hYcnIYBUTKKFDKdbGN0u8CRL7z1HNAev2TWRHTAreuFlkbNON6YpB/Zsv uQjtk3/jhR0vNQG9S+5jdAPOxEpbQeZYcmsINd5CwNJZdrLzQ8URbQUNuv1KZEQct+xR bLRPutugZclWqJmm4L5PDVVUwLX+SLO+4rhRru/rprBCb1V6nOTEJ1jMXbPU4ap9LaIn K24GXCcgtHFCtaF9ytHncrzAjw0OK1EmluVZaUpNiahzPEi4VnNiyY20QxUmrNEYbYl9 3SJw== X-Gm-Message-State: AMCzsaXAw55xuO7ZwLWTOSCDmqmSJkNBKDlLtpeq47+ySj8A70n4rjOm Zo/FRdOgMeQj/TjGJu1NxwQ= X-Google-Smtp-Source: ABhQp+QIql0Ku0SaV/sR4zuM7SU7IkN8TvB4GbmZ+Ot7vSkrsj+L6yTKP5cz4yf/K6Zgzld6sBOM/A== X-Received: by 10.107.202.3 with SMTP id a3mr20963498iog.10.1510006916656; Mon, 06 Nov 2017 14:21:56 -0800 (PST) Received: from gmail.com ([2620:15c:17:3:4d3f:4ca5:45b3:4d95]) by smtp.gmail.com with ESMTPSA id 191sm13395itb.21.2017.11.06.14.21.55 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 06 Nov 2017 14:21:56 -0800 (PST) Date: Mon, 6 Nov 2017 14:21:53 -0800 From: Eric Biggers To: David Howells Cc: syzbot , davem@davemloft.net, herbert@gondor.apana.org.au, keyrings@vger.kernel.org, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com Subject: Re: general protection fault in asn1_ber_decoder Message-ID: <20171106222153.GA37298@gmail.com> References: <94eb2c1886e4e1e95e055d54b9ab@google.com> <15884.1510005945@warthog.procyon.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <15884.1510005945@warthog.procyon.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Nov 06, 2017 at 10:05:45PM +0000, David Howells wrote: > diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c > index fef5d2e114be..048de2c20ae9 100644 > --- a/lib/asn1_decoder.c > +++ b/lib/asn1_decoder.c > @@ -201,6 +201,13 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, > if (datalen > 65535) > return -EMSGSIZE; > > + /* We don't currently support 0-length messages - the underrun checks > + * will fail if datalen is 0 because we check against datalen - 1 with > + * unsigned arithmetic. > + */ > + if (datalen == 0) > + return -EBADMSG; > + Hi David, you just beat me to it, but I don't think this is the best way to fix the problem. The length check just needs to be rewritten to not overflow. Also it seems there is another broken length check later in the function. How about this: commit 8bbe85872c660c891eb978a037f590198319e3b2 Author: Eric Biggers Date: Mon Nov 6 10:06:32 2017 -0800 KEYS: fix NULL pointer dereference during ASN.1 parsing 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 Cc: # v3.7+ Signed-off-by: Eric Biggers diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c index fef5d2e114be..1ef0cec38d78 100644 --- a/lib/asn1_decoder.c +++ b/lib/asn1_decoder.c @@ -228,7 +228,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, hdr = 2; /* Extract a tag from the data */ - if (unlikely(dp >= datalen - 1)) + if (unlikely(datalen - dp < 2)) goto data_overrun_error; tag = data[dp++]; if (unlikely((tag & 0x1f) == ASN1_LONG_TAG)) @@ -274,7 +274,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, int n = len - 0x80; if (unlikely(n > 2)) goto length_too_long; - if (unlikely(dp >= datalen - n)) + if (unlikely(n > datalen - dp)) goto data_overrun_error; hdr += n; for (len = 0; n > 0; n--) {