* [PATCH] KEYS: fix NULL pointer dereference during ASN.1 parsing
@ 2017-11-07 15:19 David Howells
2017-11-07 15:30 ` David Howells
2017-11-07 17:42 ` Eric Biggers
0 siblings, 2 replies; 3+ messages in thread
From: David Howells @ 2017-11-07 15:19 UTC (permalink / raw)
To: linux-security-module
From: Eric Biggers <ebiggers3@gmail.com>
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>
---
lib/asn1_decoder.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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--) {
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] KEYS: fix NULL pointer dereference during ASN.1 parsing
2017-11-07 15:19 [PATCH] KEYS: fix NULL pointer dereference during ASN.1 parsing David Howells
@ 2017-11-07 15:30 ` David Howells
2017-11-07 17:42 ` Eric Biggers
1 sibling, 0 replies; 3+ messages in thread
From: David Howells @ 2017-11-07 15:30 UTC (permalink / raw)
To: linux-security-module
Hi James,
Could you pass this onto Linus, please?
Thanks,
David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] KEYS: fix NULL pointer dereference during ASN.1 parsing
2017-11-07 15:19 [PATCH] KEYS: fix NULL pointer dereference during ASN.1 parsing David Howells
2017-11-07 15:30 ` David Howells
@ 2017-11-07 17:42 ` Eric Biggers
1 sibling, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2017-11-07 17:42 UTC (permalink / raw)
To: linux-security-module
On Tue, Nov 07, 2017 at 03:19:27PM +0000, David Howells wrote:
> From: Eric Biggers <ebiggers3@gmail.com>
>
> 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>
If it's not too late can you fix the From: line to have my @google.com address
to match the Signed-off-by?
From: Eric Biggers <ebiggers@google.com>
Also feel free to trim down the register dump like you asked.
Thanks!
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-07 17:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-07 15:19 [PATCH] KEYS: fix NULL pointer dereference during ASN.1 parsing David Howells
2017-11-07 15:30 ` David Howells
2017-11-07 17:42 ` Eric Biggers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).