mirror of
https://github.com/Divested-Mobile/DivestOS-Build.git
synced 2024-10-01 01:35:54 -04:00
126 lines
4.7 KiB
Diff
126 lines
4.7 KiB
Diff
From 0d62e9dd6da45bbf0f33a8617afc5fe774c8f45f Mon Sep 17 00:00:00 2001
|
|
From: David Howells <dhowells@redhat.com>
|
|
Date: Wed, 5 Aug 2015 12:54:46 +0100
|
|
Subject: ASN.1: Fix non-match detection failure on data overrun
|
|
|
|
If the ASN.1 decoder is asked to parse a sequence of objects, non-optional
|
|
matches get skipped if there's no more data to be had rather than a
|
|
data-overrun error being reported.
|
|
|
|
This is due to the code segment that decides whether to skip optional
|
|
matches (ie. matches that could get ignored because an element is marked
|
|
OPTIONAL in the grammar) due to a lack of data also skips non-optional
|
|
elements if the data pointer has reached the end of the buffer.
|
|
|
|
This can be tested with the data decoder for the new RSA akcipher algorithm
|
|
that takes three non-optional integers. Currently, it skips the last
|
|
integer if there is insufficient data.
|
|
|
|
Without the fix, #defining DEBUG in asn1_decoder.c will show something
|
|
like:
|
|
|
|
next_op: pc=0/13 dp=0/270 C=0 J=0
|
|
- match? 30 30 00
|
|
- TAG: 30 266 CONS
|
|
next_op: pc=2/13 dp=4/270 C=1 J=0
|
|
- match? 02 02 00
|
|
- TAG: 02 257
|
|
- LEAF: 257
|
|
next_op: pc=5/13 dp=265/270 C=1 J=0
|
|
- match? 02 02 00
|
|
- TAG: 02 3
|
|
- LEAF: 3
|
|
next_op: pc=8/13 dp=270/270 C=1 J=0
|
|
next_op: pc=11/13 dp=270/270 C=1 J=0
|
|
- end cons t=4 dp=270 l=270/270
|
|
|
|
The next_op line for pc=8/13 should be followed by a match line.
|
|
|
|
This is not exploitable for X.509 certificates by means of shortening the
|
|
message and fixing up the ASN.1 CONS tags because:
|
|
|
|
(1) The relevant records being built up are cleared before use.
|
|
|
|
(2) If the message is shortened sufficiently to remove the public key, the
|
|
ASN.1 parse of the RSA key will fail quickly due to a lack of data.
|
|
|
|
(3) Extracted signature data is either turned into MPIs (which cope with a
|
|
0 length) or is simpler integers specifying algoritms and suchlike
|
|
(which can validly be 0); and
|
|
|
|
(4) The AKID and SKID extensions are optional and their removal is handled
|
|
without risking passing a NULL to asymmetric_key_generate_id().
|
|
|
|
(5) If the certificate is truncated sufficiently to remove the subject,
|
|
issuer or serialNumber then the ASN.1 decoder will fail with a 'Cons
|
|
stack underflow' return.
|
|
|
|
This is not exploitable for PKCS#7 messages by means of removal of elements
|
|
from such a message from the tail end of a sequence:
|
|
|
|
(1) Any shortened X.509 certs embedded in the PKCS#7 message are survivable
|
|
as detailed above.
|
|
|
|
(2) The message digest content isn't used if it shows a NULL pointer,
|
|
similarly, the authattrs aren't used if that shows a NULL pointer.
|
|
|
|
(3) A missing signature results in a NULL MPI - which the MPI routines deal
|
|
with.
|
|
|
|
(4) If data is NULL, it is expected that the message has detached content and
|
|
that is handled appropriately.
|
|
|
|
(5) If the serialNumber is excised, the unconditional action associated
|
|
with it will pick up the containing SEQUENCE instead, so no NULL
|
|
pointer will be seen here.
|
|
|
|
If both the issuer and the serialNumber are excised, the ASN.1 decode
|
|
will fail with an 'Unexpected tag' return.
|
|
|
|
In either case, there's no way to get to asymmetric_key_generate_id()
|
|
with a NULL pointer.
|
|
|
|
(6) Other fields are decoded to simple integers. Shortening the message
|
|
to omit an algorithm ID field will cause checks on this to fail early
|
|
in the verification process.
|
|
|
|
|
|
This can also be tested by snipping objects off of the end of the ASN.1 stream
|
|
such that mandatory tags are removed - or even from the end of internal
|
|
SEQUENCEs. If any mandatory tag is missing, the error EBADMSG *should* be
|
|
produced. Without this patch ERANGE or ENOPKG might be produced or the parse
|
|
may apparently succeed, perhaps with ENOKEY or EKEYREJECTED being produced
|
|
later, depending on what gets snipped.
|
|
|
|
Just snipping off the final BIT_STRING or OCTET_STRING from either sample
|
|
should be a start since both are mandatory and neither will cause an EBADMSG
|
|
without the patches
|
|
|
|
Reported-by: Marcel Holtmann <marcel@holtmann.org>
|
|
Signed-off-by: David Howells <dhowells@redhat.com>
|
|
Tested-by: Marcel Holtmann <marcel@holtmann.org>
|
|
Reviewed-by: David Woodhouse <David.Woodhouse@intel.com>
|
|
---
|
|
lib/asn1_decoder.c | 5 ++---
|
|
1 file changed, 2 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
|
|
index 55980d7..3f74dd3 100644
|
|
--- a/lib/asn1_decoder.c
|
|
+++ b/lib/asn1_decoder.c
|
|
@@ -210,9 +210,8 @@ next_op:
|
|
unsigned char tmp;
|
|
|
|
/* Skip conditional matches if possible */
|
|
- if ((op & ASN1_OP_MATCH__COND &&
|
|
- flags & FLAG_MATCHED) ||
|
|
- dp == datalen) {
|
|
+ if ((op & ASN1_OP_MATCH__COND && flags & FLAG_MATCHED) ||
|
|
+ (op & ASN1_OP_MATCH__SKIP && dp == datalen)) {
|
|
flags &= ~FLAG_LAST_MATCHED;
|
|
pc += asn1_op_lengths[op];
|
|
goto next_op;
|
|
--
|
|
cgit v1.1
|
|
|