فهرست منبع

LibTLS: Don't attempt to read past EOF when parsing TBSCertificate

This allows the decoder to fail gracefully when reading a partial or
malformed TBSCertificate. We also now ensure that the certificate data
is valid before making a copy of it.
Tim Ledbetter 1 سال پیش
والد
کامیت
e6d9bb0774

+ 1 - 0
Meta/Lagom/CMakeLists.txt

@@ -655,6 +655,7 @@ if (BUILD_LAGOM)
 
         # LibTLS needs a special working directory to find cacert.pem
         lagom_test(../../Tests/LibTLS/TestTLSHandshake.cpp LibTLS LIBS LibTLS LibCrypto)
+        lagom_test(../../Tests/LibTLS/TestTLSCertificateParser.cpp LibTLS LIBS LibTLS)
 
         # The FLAC tests need a special working directory to find the test files
         lagom_test(../../Tests/LibAudio/TestFLACSpec.cpp LIBS LibAudio WORKING_DIRECTORY "${FLAC_TEST_PATH}/..")

+ 1 - 0
Tests/LibTLS/CMakeLists.txt

@@ -1,4 +1,5 @@
 set(TEST_SOURCES
+    TestTLSCertificateParser.cpp
     TestTLSHandshake.cpp
 )
 

+ 15 - 0
Tests/LibTLS/TestTLSCertificateParser.cpp

@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2023, Tim Ledbetter  <timledbetter@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibTLS/Certificate.h>
+#include <LibTest/TestCase.h>
+
+TEST_CASE(certificate_with_malformed_tbscertificate_should_fail_gracefully)
+{
+    Array<u8, 4> invalid_certificate_data { 0xB0, 0x02, 0x70, 0x00 };
+    auto parse_result = TLS::Certificate::parse_certificate(invalid_certificate_data);
+    EXPECT(parse_result.is_error());
+}

+ 4 - 2
Userland/Libraries/LibTLS/Certificate.cpp

@@ -691,10 +691,11 @@ static ErrorOr<Certificate> parse_tbs_certificate(Crypto::ASN1::Decoder& decoder
     ENTER_TYPED_SCOPE(Sequence, "TBSCertificate"sv);
 
     auto post_cert_buffer = TRY(decoder.peek_entry_bytes());
-    auto asn1_data = TRY(ByteBuffer::copy(pre_cert_buffer.slice(0, post_cert_buffer.size() + entry_length_byte_count)));
+    if (pre_cert_buffer.size() < post_cert_buffer.size() + entry_length_byte_count) {
+        ERROR_WITH_SCOPE("Unexpected end of file");
+    }
 
     Certificate certificate;
-    certificate.tbs_asn1 = asn1_data;
     certificate.version = TRY(parse_certificate_version(decoder, current_scope)).to_u64();
     certificate.serial_number = TRY(parse_serial_number(decoder, current_scope));
     certificate.algorithm = TRY(parse_algorithm_identifier(decoder, current_scope));
@@ -702,6 +703,7 @@ static ErrorOr<Certificate> parse_tbs_certificate(Crypto::ASN1::Decoder& decoder
     certificate.validity = TRY(parse_validity(decoder, current_scope));
     certificate.subject = TRY(parse_name(decoder, current_scope));
     certificate.public_key = TRY(parse_subject_public_key_info(decoder, current_scope));
+    certificate.tbs_asn1 = TRY(ByteBuffer::copy(pre_cert_buffer.slice(0, post_cert_buffer.size() + entry_length_byte_count)));
 
     if (!decoder.eof()) {
         auto tag = TRY(decoder.peek());