[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 <mcb30@ipxe.org>

diff --git a/src/crypto/asn1.c b/src/crypto/asn1.c
index ec4712f6bfce6f9333f0792d9faf1113ae012d2b..90619dc0ec6aafc7189f6a77d048412bcb19fe0e 100644 (file)
--- 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;