Skip to content

Commit dc5b223

Browse files
Reject len < -1 in ASN1_mbstring_ncopy (#3232)
### Issues: Addresses `V2196133741` ### Description of changes: `ASN1_mbstring_ncopy` treats `len == -1` as "call strlen on in". Any other negative value fell through unchanged and was cast to `size_t` by `CBS_init`, producing a huge length. Adds an explicit early reject for `len < -1` with `ASN1_R_ILLEGAL_FORMAT`; `-1` and non-negative values continue to behave as before. ### Call-outs: None. ### Testing: Existing ASN.1 string tests continue to pass. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
1 parent 25a859c commit dc5b223

2 files changed

Lines changed: 23 additions & 0 deletions

File tree

crypto/asn1/a_mbstr.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ OPENSSL_DECLARE_ERROR_REASON(ASN1, INVALID_UTF8STRING)
3333
int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in,
3434
ossl_ssize_t len, int inform, unsigned long mask,
3535
ossl_ssize_t minsize, ossl_ssize_t maxsize) {
36+
if (len < -1) {
37+
OPENSSL_PUT_ERROR(ASN1, ASN1_R_ILLEGAL_FORMAT);
38+
return -1;
39+
}
3640
if (len == -1) {
3741
len = strlen((const char *)in);
3842
}

crypto/asn1/asn1_test.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,6 +1711,25 @@ TEST(ASN1Test, MBString) {
17111711
ERR_clear_error();
17121712
EXPECT_EQ(nullptr, str);
17131713
}
1714+
1715+
// |len| values below -1 must be rejected; only -1 is special-cased to mean
1716+
// "call strlen on |in|". Any other negative value would otherwise be cast
1717+
// to a huge |size_t| by |CBS_init|.
1718+
static const uint8_t kDummy[] = {'a'};
1719+
ASN1_STRING *str = nullptr;
1720+
EXPECT_EQ(-1, ASN1_mbstring_ncopy(&str, kDummy, -2, MBSTRING_UTF8,
1721+
B_ASN1_UTF8STRING, /*minsize=*/0,
1722+
/*maxsize=*/0));
1723+
EXPECT_EQ(nullptr, str);
1724+
ERR_clear_error();
1725+
1726+
// |len == -1| treats |in| as NUL-terminated and should succeed.
1727+
ASN1_STRING *str_from_strlen = nullptr;
1728+
EXPECT_GE(ASN1_mbstring_ncopy(&str_from_strlen, (const uint8_t *)"a", -1,
1729+
MBSTRING_UTF8, B_ASN1_UTF8STRING,
1730+
/*minsize=*/0, /*maxsize=*/0),
1731+
0);
1732+
ASN1_STRING_free(str_from_strlen);
17141733
}
17151734

17161735
TEST(ASN1Test, StringByNID) {

0 commit comments

Comments
 (0)