From 0ca41d2813838872c469b11c9831d994bde4787b Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 11 Sep 2022 21:39:29 +0200 Subject: LibCrypto: Don't crash in ASN1::parse_utc_time on missing 'Z' The underlying reason is an unconditional call to consume(), even if there is no reason to expect that the string continues. This crash was discovered by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=42354 This bug exists since the code was first written in April 2021: 13abbc5ea8c6b0bb145983c7d232dbe3bdc398c3 --- Tests/LibCrypto/TestASN1.cpp | 11 +++++------ Userland/Libraries/LibCrypto/ASN1/ASN1.cpp | 31 +++++++++++++++--------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/Tests/LibCrypto/TestASN1.cpp b/Tests/LibCrypto/TestASN1.cpp index 6c766b787e..e18b61174d 100644 --- a/Tests/LibCrypto/TestASN1.cpp +++ b/Tests/LibCrypto/TestASN1.cpp @@ -57,10 +57,9 @@ TEST_CASE(test_utc_missing_z) // YYMMDDhhmm[ss] // We don't actually need to parse this correctly; rejecting these inputs is fine. // This test just makes sure that we don't crash. - // FIXME: This currently crashes - // (void)Crypto::ASN1::parse_utc_time("010101010101"sv); - // (void)Crypto::ASN1::parse_utc_time("010203040506"sv); - // (void)Crypto::ASN1::parse_utc_time("020406081012"sv); - // (void)Crypto::ASN1::parse_utc_time("0204060810"sv); - // (void)Crypto::ASN1::parse_utc_time("220911220000"sv); + (void)Crypto::ASN1::parse_utc_time("010101010101"sv); + (void)Crypto::ASN1::parse_utc_time("010203040506"sv); + (void)Crypto::ASN1::parse_utc_time("020406081012"sv); + (void)Crypto::ASN1::parse_utc_time("0204060810"sv); + (void)Crypto::ASN1::parse_utc_time("220911220000"sv); } diff --git a/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp b/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp index 8acf1f37f0..a585646293 100644 --- a/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp +++ b/Userland/Libraries/LibCrypto/ASN1/ASN1.cpp @@ -84,26 +84,25 @@ Optional parse_utc_time(StringView time) auto minute = lexer.consume(2).to_uint(); Optional seconds, offset_hours, offset_minutes; [[maybe_unused]] bool negative_offset = false; - if (!lexer.next_is('Z')) { - if (!lexer.next_is(is_any_of("+-"sv))) { - seconds = lexer.consume(2).to_uint(); - if (!seconds.has_value()) { - return {}; - } + + if (lexer.next_is(is_any_of("0123456789"sv))) { + seconds = lexer.consume(2).to_uint(); + if (!seconds.has_value()) { + return {}; } + } - if (lexer.next_is(is_any_of("+-"sv))) { - negative_offset = lexer.consume() == '-'; - offset_hours = lexer.consume(2).to_uint(); - offset_minutes = lexer.consume(2).to_uint(); - if (!offset_hours.has_value() || !offset_minutes.has_value()) { - return {}; - } - } else { - lexer.consume(); + if (lexer.next_is('Z')) { + lexer.consume(); + } else if (lexer.next_is(is_any_of("+-"sv))) { + negative_offset = lexer.consume() == '-'; + offset_hours = lexer.consume(2).to_uint(); + offset_minutes = lexer.consume(2).to_uint(); + if (!offset_hours.has_value() || !offset_minutes.has_value()) { + return {}; } } else { - lexer.consume(); + return {}; } if (!year_in_century.has_value() || !month.has_value() || !day.has_value() || !hour.has_value() || !minute.has_value()) { -- cgit v1.2.3