Fix a copy-paste bug in ASN1_TIME_compare()

ASN1_TIME_compare() compares two times t1 and t2. Due to a copy-paste
error, we would do ASN1_time_parse(t1->data, t2->length, &tm2, t2->type)

Now if t1 is a UTCTime (length 13) and t2 is a GeneralizedTime (length 15),
the worst that could happen is a 2-byte out-of-bounds read. Fortunately, t1
will already have parsed as a UTCTime, so it will have a Z where there
should be the first digit of the seconds for a GeneralizedTime and we will
error out.

Now if both t1 and t2 have the same type, we will parse t1's data twice
and we will return an incorrect comparison. This could have some security
impact if anything relied on this function for security purposes. It is
unused in our tree and unused in our ports tree ports and the only consumer
I could find was some MongoDB things doing OCSP, so this won't be too bad.

Then of course there's also the language bindings.

Issue reported by Duncan Thomson at esri dot com via libressl-security

ok beck deraadt

diff --git a/lib/libcrypto/asn1/a_time_tm.c b/lib/libcrypto/asn1/a_time_tm.c
index 556e12a..ea94d2f 100644 (file)
--- a/lib/libcrypto/asn1/a_time_tm.c
+++ b/lib/libcrypto/asn1/a_time_tm.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_time_tm.c,v 1.30 2023/08/30 10:13:12 job Exp $ */
+/* $OpenBSD: a_time_tm.c,v 1.31 2023/10/01 22:14:36 tb Exp $ */
 /*
  * Copyright (c) 2015 Bob Beck <beck@openbsd.org>
  *
@@ -503,7 +503,7 @@ ASN1_TIME_compare(const ASN1_TIME *t1, const ASN1_TIME *t2)
        if (ASN1_time_parse(t1->data, t1->length, &tm1, t1->type) == -1)
                return -2;
 
-       if (ASN1_time_parse(t1->data, t2->length, &tm2, t2->type) == -1)
+       if (ASN1_time_parse(t2->data, t2->length, &tm2, t2->type) == -1)
                return -2;
 
        return ASN1_time_tm_cmp(&tm1, &tm2);