From 4fc3ccbc6eeb59b163d90b255eba7961c2a33ffb Mon Sep 17 00:00:00 2001 From: schwarze Date: Fri, 19 Nov 2021 09:58:41 +0000 Subject: [PATCH] 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@ --- lib/libcrypto/asn1/f_string.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) 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; -- 2.20.1