Fix error-handling for i2a_ASN1_OBJECT. Some BIO_write failures weren't handled. Otherwise would successfully write truncated results. The other i2a functions all report -1 on truncation, so match. While I'm here, write a test to make sure I didn't break this. Change-Id: If17d0209e75c15b3f37bceb1cdfb480fd2c62c4d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49931 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c index b88f06b..0de3141 100644 --- a/crypto/asn1/a_object.c +++ b/crypto/asn1/a_object.c
@@ -110,26 +110,37 @@ return OBJ_obj2txt(buf, buf_len, a, 0); } +static int write_str(BIO *bp, const char *str) +{ + int len = strlen(str); + return BIO_write(bp, str, len) == len ? len : -1; +} + int i2a_ASN1_OBJECT(BIO *bp, const ASN1_OBJECT *a) { - char buf[80], *p = buf; - int i; - - if ((a == NULL) || (a->data == NULL)) - return (BIO_write(bp, "NULL", 4)); - i = i2t_ASN1_OBJECT(buf, sizeof buf, a); - if (i > (int)(sizeof(buf) - 1)) { - p = OPENSSL_malloc(i + 1); - if (!p) - return -1; - i2t_ASN1_OBJECT(p, i + 1, a); + if (a == NULL || a->data == NULL) { + return write_str(bp, "NULL"); } - if (i <= 0) - return BIO_write(bp, "<INVALID>", 9); - BIO_write(bp, p, i); - if (p != buf) - OPENSSL_free(p); - return (i); + + char buf[80], *allocated = NULL; + const char *str = buf; + int len = i2t_ASN1_OBJECT(buf, sizeof(buf), a); + if (len > (int)sizeof(buf) - 1) { + /* The input was truncated. Allocate a buffer that fits. */ + allocated = OPENSSL_malloc(len + 1); + if (allocated == NULL) { + return -1; + } + len = i2t_ASN1_OBJECT(allocated, len + 1, a); + str = allocated; + } + if (len <= 0) { + str = "<INVALID>"; + } + + int ret = write_str(bp, str); + OPENSSL_free(allocated); + return ret; } ASN1_OBJECT *d2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc index bb2b0a8..1381169 100644 --- a/crypto/asn1/asn1_test.cc +++ b/crypto/asn1/asn1_test.cc
@@ -1587,6 +1587,88 @@ } } +TEST(ASN1Test, PrintASN1Object) { + const struct { + std::vector<uint8_t> in; + const char *expected; + } kDataTests[] = { + // Known OIDs print as the name. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01}, "rsaEncryption"}, + + // Unknown OIDs print in decimal. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, 0x09, 0x00}, + "1.2.840.113554.4.1.72585.0"}, + + // Inputs which cannot be parsed as OIDs print as "<INVALID>". + {{0xff}, "<INVALID>"}, + + // The function has an internal 80-byte buffer. Test inputs at that + // boundary. First, 78 characters. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, + 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, + "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0." + "0.0.0.1"}, + // 79 characters. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, + 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a}, + "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0." + "0.0.0.10"}, + // 80 characters. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, + 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64}, + "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0." + "0.0.0.100"}, + // 81 characters. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, + 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x87, 0x68}, + "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0." + "0.0.0.1000"}, + // 82 characters. + {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, + 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xce, 0x10}, + "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0." + "0.0.0.10000"}, + }; + for (const auto &t : kDataTests) { + SCOPED_TRACE(Bytes(t.in)); + bssl::UniquePtr<ASN1_OBJECT> obj(ASN1_OBJECT_create( + NID_undef, t.in.data(), t.in.size(), /*sn=*/nullptr, /*ln=*/nullptr)); + ASSERT_TRUE(obj); + bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem())); + ASSERT_TRUE(bio); + + int len = i2a_ASN1_OBJECT(bio.get(), obj.get()); + EXPECT_EQ(len, static_cast<int>(strlen(t.expected))); + + const uint8_t *bio_data; + size_t bio_len; + BIO_mem_contents(bio.get(), &bio_data, &bio_len); + EXPECT_EQ(t.expected, + std::string(reinterpret_cast<const char *>(bio_data), bio_len)); + } + + // Test writing NULL. + bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem())); + ASSERT_TRUE(bio); + int len = i2a_ASN1_OBJECT(bio.get(), nullptr); + EXPECT_EQ(len, 4); + const uint8_t *bio_data; + size_t bio_len; + BIO_mem_contents(bio.get(), &bio_data, &bio_len); + EXPECT_EQ("NULL", + std::string(reinterpret_cast<const char *>(bio_data), bio_len)); +} + // The ASN.1 macros do not work on Windows shared library builds, where usage of // |OPENSSL_EXPORT| is a bit stricter. #if !defined(OPENSSL_WINDOWS) || !defined(BORINGSSL_SHARED_LIBRARY)