From ae4766ae96d0dcb5167492ec9e2b985d42324e84 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Fri, 10 Sep 2021 16:49:42 -0600 Subject: [PATCH 1/3] add sanity check on buffer size --- wolfcrypt/src/asn.c | 4 ++++ wolfcrypt/test/test.c | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 69f03006a..c27c18b84 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -26934,6 +26934,10 @@ static int DecodeAsymKeyPublic(const byte* input, word32* inOutIdx, word32 inSz, if (ret != 0) return ret; + /* check that the value found is not too large for pubKey buffer */ + if (inSz - *inOutIdx > *pubKeyLen) + return ASN_PARSE_E; + /* This is the raw point data compressed or uncompressed. */ *pubKeyLen = inSz - *inOutIdx; XMEMCPY(pubKey, input + *inOutIdx, *pubKeyLen); diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 0247bbb8e..2b0814b35 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -26031,6 +26031,17 @@ WOLFSSL_TEST_SUBROUTINE int ed25519_test(void) 0xda,0xa6,0x23,0x25,0xaf,0x02,0x1a,0x68, 0xf7,0x07,0x51,0x1a }; + + /* size has been altered to catch if sanity check is done */ + static byte badPublicEd25519[] = { + 0x30,0x2a,0x30,0x05,0x06,0x03,0x2b,0x65, + 0x70,0x03,0x21,0x00,0xd7,0x5a,0x98,0x01, + 0x82,0xb1,0x0a,0xb7,0xd5,0x4b,0xfe,0xd3, + 0xc9,0x64,0x07,0x3a,0x0e,0xe1,0x72,0xf3, + 0xda,0xa6,0x23,0x25,0xaf,0x02,0x1a,0x68, + 0xf7,0x07,0x51,0x1a, + 0x00 /* add an additional byte to make the pubkey appear bigger */ + }; static byte privPubEd25519[] = { 0x30,0x52,0x02,0x01,0x00,0x30,0x05,0x06, 0x03,0x2b,0x65,0x70,0x04,0x22,0x04,0x20, @@ -26167,6 +26178,13 @@ WOLFSSL_TEST_SUBROUTINE int ed25519_test(void) != BAD_FUNC_ARG) return -11131; + + /* try with a buffer size that is too large */ + idx = 0; + if (wc_Ed25519PublicKeyDecode(badPublicEd25519, &idx, &key3, + sizeof(badPublicEd25519)) == 0) + return -11140; + idx = 0; if (wc_Ed25519PublicKeyDecode(publicEd25519, &idx, &key3, sizeof(publicEd25519)) != 0) From 602ec188ad9a2508e8463c29f5d8783a8417ace4 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Fri, 10 Sep 2021 21:51:18 -0600 Subject: [PATCH 2/3] sanity checks on ed25519 private key decode --- wolfcrypt/src/asn.c | 7 +++++++ wolfcrypt/test/test.c | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index c27c18b84..c6c73d702 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -26821,6 +26821,9 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz, if (GetOctetString(input, inOutIdx, &privSz, inSz) < 0) return ASN_PARSE_E; + if ((word32)privSz > *privKeyLen) + return BUFFER_E; + priv = input + *inOutIdx; *inOutIdx += privSz; endKeyIdx = *inOutIdx; @@ -26840,6 +26843,10 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz, if (GetOctetString(input, inOutIdx, &pubSz, inSz) < 0) { return ASN_PARSE_E; } + + if ((word32)pubSz > *pubKeyLen) + return BUFFER_E; + pub = input + *inOutIdx; *inOutIdx += pubSz; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 2b0814b35..30c698db9 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -26023,6 +26023,20 @@ WOLFSSL_TEST_SUBROUTINE int ed25519_test(void) 0x44,0x49,0xc5,0x69,0x7b,0x32,0x69,0x19, 0x70,0x3b,0xac,0x03,0x1c,0xae,0x7f,0x60 }; + static byte badPrivateEd25519[] = { + 0x30,0x52,0x02,0x01,0x00,0x30,0x05,0x06, + 0x03,0x2b,0x65,0x70,0x04,0x22,0x04,0x20, + 0x9d,0x61,0xb1,0x9d,0xef,0xfd,0x5a,0x60, + 0xba,0x84,0x4a,0xf4,0x92,0xec,0x2c,0xc4, + 0x44,0x49,0xc5,0x69,0x7b,0x32,0x69,0x19, + 0x70,0x3b,0xac,0x03,0x1c,0xae,0x7f,0x60, + 0xa1,0x22,0x04,0x21,0xd7,0x5a,0x98,0x01, /* octet len 0x20 -> 0x21 */ + 0x82,0xb1,0x0a,0xb7,0xd5,0x4b,0xfe,0xd3, + 0xc9,0x64,0x07,0x3a,0x0e,0xe1,0x72,0xf3, + 0xda,0xa6,0x23,0x25,0xaf,0x02,0x1a,0x68, + 0xf7,0x07,0x51,0x1a, + 0x00 /* add additional bytes to make the pubkey bigger */ + }; static byte publicEd25519[] = { 0x30,0x2a,0x30,0x05,0x06,0x03,0x2b,0x65, 0x70,0x03,0x21,0x00,0xd7,0x5a,0x98,0x01, @@ -26174,6 +26188,11 @@ WOLFSSL_TEST_SUBROUTINE int ed25519_test(void) sizeof(privateEd25519)) != 0) return -11121; + idx = 0; + if (wc_Ed25519PrivateKeyDecode(badPrivateEd25519, &idx, &key3, + sizeof(badPrivateEd25519)) == 0) + return -11122; + if (wc_ed25519_sign_msg(msgs[0], msgSz[0], out, &outlen, &key3) != BAD_FUNC_ARG) return -11131; From f06414903cff45af37fb936996cd4b4247599e0e Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 13 Sep 2021 09:35:55 -0600 Subject: [PATCH 3/3] fix for scan build warning and better check on size --- wolfcrypt/src/asn.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index c6c73d702..2a04a4a95 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -26821,14 +26821,14 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz, if (GetOctetString(input, inOutIdx, &privSz, inSz) < 0) return ASN_PARSE_E; - if ((word32)privSz > *privKeyLen) - return BUFFER_E; - priv = input + *inOutIdx; *inOutIdx += privSz; endKeyIdx = *inOutIdx; } + if ((word32)privSz > *privKeyLen) + return BUFFER_E; + if (endKeyIdx == (int)*inOutIdx) { *privKeyLen = privSz; XMEMCPY(privKey, priv, *privKeyLen); @@ -26836,6 +26836,10 @@ static int DecodeAsymKey(const byte* input, word32* inOutIdx, word32 inSz, *pubKeyLen = 0; } else { + if (pubKeyLen == NULL) { + return BAD_FUNC_ARG; + } + if (GetASNHeader(input, ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | 1, inOutIdx, &length, inSz) < 0) { return ASN_PARSE_E;