From 0e73b48f77589f69a5418c211b77b95ed26520c8 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 7 Aug 2024 13:36:35 +0100 Subject: [crypto] Clarify ASN.1 cursor invalidation behaviour Calling asn1_skip_if_exists() on a malformed ASN.1 object may currently leave the cursor in a partially-updated state, where the tag byte and one of the length bytes have been stripped. The cursor is left with a valid data pointer and length and so no out-of-bounds access can arise, but the cursor no longer points to the start of an ASN.1 object. Ensure that each ASN.1 cursor manipulation code path leads to the cursor being either fully updated, left unmodified, or invalidated, and update the function descriptions to reflect this. Signed-off-by: Michael Brown --- src/crypto/asn1.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/crypto/asn1.c b/src/crypto/asn1.c index ec4712f6b..90619dc0e 100644 --- a/src/crypto/asn1.c +++ b/src/crypto/asn1.c @@ -94,6 +94,10 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); * object body (i.e. the first byte following the length byte(s)), and * the length of the object body (i.e. the number of bytes until the * following object tag, if any) is returned. + * + * If the expected type is not found, the object cursor will not be + * modified. If any other error occurs, the object cursor will be + * invalidated. */ int asn1_start ( struct asn1_cursor *cursor, unsigned int type, size_t extra ) { unsigned int len_len; @@ -103,6 +107,7 @@ int asn1_start ( struct asn1_cursor *cursor, unsigned int type, size_t extra ) { if ( cursor->len < 2 /* Tag byte and first length byte */ ) { if ( cursor->len ) DBGC ( cursor, "ASN1 %p too short\n", cursor ); + asn1_invalidate_cursor ( cursor ); return -EINVAL_ASN1_EMPTY; } @@ -127,6 +132,7 @@ int asn1_start ( struct asn1_cursor *cursor, unsigned int type, size_t extra ) { if ( cursor->len < len_len ) { DBGC ( cursor, "ASN1 %p bad length field length %d (max " "%zd)\n", cursor, len_len, cursor->len ); + asn1_invalidate_cursor ( cursor ); return -EINVAL_ASN1_LEN_LEN; } @@ -140,6 +146,7 @@ int asn1_start ( struct asn1_cursor *cursor, unsigned int type, size_t extra ) { if ( ( cursor->len + extra ) < len ) { DBGC ( cursor, "ASN1 %p bad length %d (max %zd)\n", cursor, len, ( cursor->len + extra ) ); + asn1_invalidate_cursor ( cursor ); return -EINVAL_ASN1_LEN; } @@ -154,8 +161,9 @@ int asn1_start ( struct asn1_cursor *cursor, unsigned int type, size_t extra ) { * @ret rc Return status code * * The object cursor will be updated to point to the body of the - * current ASN.1 object. If any error occurs, the object cursor will - * be invalidated. + * current ASN.1 object. + * + * If any error occurs, the object cursor will be invalidated. */ int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) { int len; @@ -181,8 +189,11 @@ int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) { * @ret rc Return status code * * The object cursor will be updated to point to the next ASN.1 - * object. If any error occurs, the object cursor will not be - * modified. + * object. + * + * If the expected type is not found, the object cursor will not be + * modified. If any other error occurs, the object cursor will be + * invalidated. */ int asn1_skip_if_exists ( struct asn1_cursor *cursor, unsigned int type ) { int len; @@ -207,8 +218,9 @@ int asn1_skip_if_exists ( struct asn1_cursor *cursor, unsigned int type ) { * @ret rc Return status code * * The object cursor will be updated to point to the next ASN.1 - * object. If any error occurs, the object cursor will be - * invalidated. + * object. + * + * If any error occurs, the object cursor will be invalidated. */ int asn1_skip ( struct asn1_cursor *cursor, unsigned int type ) { int rc; @@ -229,8 +241,9 @@ int asn1_skip ( struct asn1_cursor *cursor, unsigned int type ) { * @ret rc Return status code * * The object cursor will be shrunk to contain only the current ASN.1 - * object. If any error occurs, the object cursor will be - * invalidated. + * object. + * + * If any error occurs, the object cursor will be invalidated. */ int asn1_shrink ( struct asn1_cursor *cursor, unsigned int type ) { struct asn1_cursor temp; -- cgit