Skip to content

Commit e0058f3

Browse files
ebiggersdhowells
authored andcommitted
ASN.1: fix out-of-bounds read when parsing indefinite length item
In asn1_ber_decoder(), indefinitely-sized ASN.1 items were being passed to the action functions before their lengths had been computed, using the bogus length of 0x80 (ASN1_INDEFINITE_LENGTH). This resulted in reading data past the end of the input buffer, when given a specially crafted message. Fix it by rearranging the code so that the indefinite length is resolved before the action is called. This bug was originally found by fuzzing the X.509 parser in userspace using libFuzzer from the LLVM project. KASAN report (cleaned up slightly): BUG: KASAN: slab-out-of-bounds in memcpy ./include/linux/string.h:341 [inline] BUG: KASAN: slab-out-of-bounds in x509_fabricate_name.constprop.1+0x1a4/0x940 crypto/asymmetric_keys/x509_cert_parser.c:366 Read of size 128 at addr ffff880035dd9eaf by task keyctl/195 CPU: 1 PID: 195 Comm: keyctl Not tainted 4.14.0-09238-g1d3b78bbc6e9 #26 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0xd1/0x175 lib/dump_stack.c:53 print_address_description+0x78/0x260 mm/kasan/report.c:252 kasan_report_error mm/kasan/report.c:351 [inline] kasan_report+0x23f/0x350 mm/kasan/report.c:409 memcpy+0x1f/0x50 mm/kasan/kasan.c:302 memcpy ./include/linux/string.h:341 [inline] x509_fabricate_name.constprop.1+0x1a4/0x940 crypto/asymmetric_keys/x509_cert_parser.c:366 asn1_ber_decoder+0xb4a/0x1fd0 lib/asn1_decoder.c:447 x509_cert_parse+0x1c7/0x620 crypto/asymmetric_keys/x509_cert_parser.c:89 x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174 asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388 key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850 SYSC_add_key security/keys/keyctl.c:122 [inline] SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62 entry_SYSCALL_64_fastpath+0x1f/0x96 Allocated by task 195: __do_kmalloc_node mm/slab.c:3675 [inline] __kmalloc_node+0x47/0x60 mm/slab.c:3682 kvmalloc ./include/linux/mm.h:540 [inline] SYSC_add_key security/keys/keyctl.c:104 [inline] SyS_add_key+0x19e/0x290 security/keys/keyctl.c:62 entry_SYSCALL_64_fastpath+0x1f/0x96 Fixes: 42d5ec2 ("X.509: Add an ASN.1 decoder") Reported-by: Alexander Potapenko <[email protected]> Cc: <[email protected]> # v3.7+ Signed-off-by: Eric Biggers <[email protected]> Signed-off-by: David Howells <[email protected]>
1 parent 4dca6ea commit e0058f3

File tree

1 file changed

+26
-21
lines changed

1 file changed

+26
-21
lines changed

lib/asn1_decoder.c

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -313,42 +313,47 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
313313

314314
/* Decide how to handle the operation */
315315
switch (op) {
316-
case ASN1_OP_MATCH_ANY_ACT:
317-
case ASN1_OP_MATCH_ANY_ACT_OR_SKIP:
318-
case ASN1_OP_COND_MATCH_ANY_ACT:
319-
case ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP:
320-
ret = actions[machine[pc + 1]](context, hdr, tag, data + dp, len);
321-
if (ret < 0)
322-
return ret;
323-
goto skip_data;
324-
325-
case ASN1_OP_MATCH_ACT:
326-
case ASN1_OP_MATCH_ACT_OR_SKIP:
327-
case ASN1_OP_COND_MATCH_ACT_OR_SKIP:
328-
ret = actions[machine[pc + 2]](context, hdr, tag, data + dp, len);
329-
if (ret < 0)
330-
return ret;
331-
goto skip_data;
332-
333316
case ASN1_OP_MATCH:
334317
case ASN1_OP_MATCH_OR_SKIP:
318+
case ASN1_OP_MATCH_ACT:
319+
case ASN1_OP_MATCH_ACT_OR_SKIP:
335320
case ASN1_OP_MATCH_ANY:
336321
case ASN1_OP_MATCH_ANY_OR_SKIP:
322+
case ASN1_OP_MATCH_ANY_ACT:
323+
case ASN1_OP_MATCH_ANY_ACT_OR_SKIP:
337324
case ASN1_OP_COND_MATCH_OR_SKIP:
325+
case ASN1_OP_COND_MATCH_ACT_OR_SKIP:
338326
case ASN1_OP_COND_MATCH_ANY:
339327
case ASN1_OP_COND_MATCH_ANY_OR_SKIP:
340-
skip_data:
328+
case ASN1_OP_COND_MATCH_ANY_ACT:
329+
case ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP:
330+
341331
if (!(flags & FLAG_CONS)) {
342332
if (flags & FLAG_INDEFINITE_LENGTH) {
333+
size_t tmp = dp;
334+
343335
ret = asn1_find_indefinite_length(
344-
data, datalen, &dp, &len, &errmsg);
336+
data, datalen, &tmp, &len, &errmsg);
345337
if (ret < 0)
346338
goto error;
347-
} else {
348-
dp += len;
349339
}
350340
pr_debug("- LEAF: %zu\n", len);
351341
}
342+
343+
if (op & ASN1_OP_MATCH__ACT) {
344+
unsigned char act;
345+
346+
if (op & ASN1_OP_MATCH__ANY)
347+
act = machine[pc + 1];
348+
else
349+
act = machine[pc + 2];
350+
ret = actions[act](context, hdr, tag, data + dp, len);
351+
if (ret < 0)
352+
return ret;
353+
}
354+
355+
if (!(flags & FLAG_CONS))
356+
dp += len;
352357
pc += asn1_op_lengths[op];
353358
goto next_op;
354359

0 commit comments

Comments
 (0)