From: schwarze <schwarze@openbsd.org>
Date: Fri, 19 Nov 2021 09:58:41 +0000 (+0000)
Subject: Make the public API function a2i_ASN1_STRING(3) actually work.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4fc3ccbc6eeb59b163d90b255eba7961c2a33ffb;p=openbsd

Make the public API function a2i_ASN1_STRING(3) actually work.
It contained two bugs:

1. If an input line ended in a backslash requesting line continuation,
there was duplicate code for removing that backslash, erroneously
removing another byte from the input and often causing the function
to return failure instead of correctly parsing valid input.

2. According to a comment in the source code, the former big "for"
loop was intended to "clear all the crap off the end of the line",
but actually, if there were multiple characters on the line that
were not hexadecimal digits, only the last of those and everything
following it was deleted, while all the earlier ones remained.
Besides, code further down clearly intends to error out when there
are invalid characters, which makes no sense if earlier code already
deletes such characters.  Hence the comment did not only contradict
the code above it - but contradicted the code below it, too.

Resolve these contradiction in favour of stricter parsing:
No longer skip invalid characters but always error out
when any are found.

OK & "Unbelievable" tb@
---

diff --git a/lib/libcrypto/asn1/f_string.c b/lib/libcrypto/asn1/f_string.c
index af17f43e1d1..b34343db390 100644
--- a/lib/libcrypto/asn1/f_string.c
+++ b/lib/libcrypto/asn1/f_string.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: f_string.c,v 1.18 2018/04/25 11:48:21 tb Exp $ */
+/* $OpenBSD: f_string.c,v 1.19 2021/11/19 09:58:41 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -125,26 +125,18 @@ a2i_ASN1_STRING(BIO *bp, ASN1_STRING *bs, char *buf, int size)
 			buf[--i] = '\0';
 		if (i == 0)
 			goto err_sl;
-		again = (buf[i - 1] == '\\');
-
-		for (j = i - 1; j > 0; j--) {
-			if (!(((buf[j] >= '0') && (buf[j] <= '9')) ||
-			    ((buf[j] >= 'a') && (buf[j] <= 'f')) ||
-			    ((buf[j] >= 'A') && (buf[j] <= 'F')))) {
-				i = j;
-				break;
-			}
-		}
+		if (buf[i - 1] == '\\') {
+			i--;
+			again = 1;
+		} else
+			again = 0;
 		buf[i] = '\0';
-		/* We have now cleared all the crap off the end of the
-		 * line */
 		if (i < 2)
 			goto err_sl;
 
 		bufp = (unsigned char *)buf;
 
 		k = 0;
-		i -= again;
 		if (i % 2 != 0) {
 			ASN1error(ASN1_R_ODD_NUMBER_OF_CHARS);
 			goto err;