From d30837c170dc3afeb179ca06decefb4f58a85a1b Mon Sep 17 00:00:00 2001 From: tb Date: Sun, 1 Oct 2023 22:14:36 +0000 Subject: [PATCH] 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 --- lib/libcrypto/asn1/a_time_tm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/libcrypto/asn1/a_time_tm.c b/lib/libcrypto/asn1/a_time_tm.c index 556e12a3673..ea94d2f59db 100644 --- 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 * @@ -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); -- 2.20.1