From e4f40fb6c033a06a71f11eb034d5c9e9d12b603a Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 26 Feb 2018 13:55:56 -0700 Subject: [PATCH 1/5] add sanity checks and change index increment --- wolfcrypt/src/asn.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 88083370d..fe1bd8a72 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11039,6 +11039,10 @@ static int ASNToHexString(const byte* input, word32* inOutIdx, char** out, int i; char* str; + if (*inOutIdx >= inSz) { + return BUFFER_E; + } + if (input[*inOutIdx] == ASN_INTEGER) { if (GetASNInt(input, inOutIdx, &len, inSz) < 0) return ASN_PARSE_E; @@ -11081,6 +11085,10 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, if (ret != 0) return ret; + if (*inOutIdx >= inSz) { + return BUFFER_E; + } + if (input[*inOutIdx] == (ASN_SEQUENCE | ASN_CONSTRUCTED)) { #ifdef WOLFSSL_CUSTOM_CURVES ecc_set_type* curve; @@ -11131,7 +11139,7 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, if (input[*inOutIdx] == ASN_BIT_STRING) { len = 0; ret = GetASNHeader(input, ASN_BIT_STRING, inOutIdx, &len, inSz); - inOutIdx += len; + *inOutIdx += len; } } if (ret == 0) { From 5ef4296b3dfdb6f565abd27c08e0893611f96db6 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 26 Feb 2018 14:25:39 -0700 Subject: [PATCH 2/5] sanity check on buffer length with ASNToHexString --- wolfcrypt/src/asn.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index fe1bd8a72..2b2d7ad7b 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11032,13 +11032,19 @@ static void ByteToHex(byte n, char* str) str[1] = hexChar[n & 0xf]; } -static int ASNToHexString(const byte* input, word32* inOutIdx, char** out, - word32 inSz, void* heap, int heapType) +/* + * outLen gets set to the size of buffer malloc'd (including null terminator) + * + * returns 0 on success + */ +static int ASNToHexString(const byte* input, word32* inOutIdx, int* outLen, + char** out, word32 inSz, void* heap, int heapType) { int len; int i; char* str; + *outLen = 0; if (*inOutIdx >= inSz) { return BUFFER_E; } @@ -11058,7 +11064,8 @@ static int ASNToHexString(const byte* input, word32* inOutIdx, char** out, str[len*2] = '\0'; *inOutIdx += len; - *out = str; + *out = str; + *outLen = len * 2 + 1; return 0; } @@ -11118,8 +11125,8 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, } if (ret == 0) { SkipObjectId(input, inOutIdx, inSz); - ret = ASNToHexString(input, inOutIdx, (char**)&curve->prime, inSz, - key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->prime, + inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { curve->size = (int)XSTRLEN(curve->prime) / 2; @@ -11128,12 +11135,12 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, ret = ASN_PARSE_E; } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, (char**)&curve->Af, inSz, - key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->Af, + inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, (char**)&curve->Bf, inSz, - key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->Bf, + inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { if (input[*inOutIdx] == ASN_BIT_STRING) { @@ -11143,16 +11150,22 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, } } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, (char**)&point, inSz, + ret = ASNToHexString(input, inOutIdx, &len, (char**)&point, inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + if (ret == 0 && len < (curve->size * 4) + 2) { + XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = BUFFER_E; + } } if (ret == 0) { curve->Gx = (const char*)XMALLOC(curve->size * 2 + 2, key->heap, DYNAMIC_TYPE_ECC_BUFFER); curve->Gy = (const char*)XMALLOC(curve->size * 2 + 2, key->heap, DYNAMIC_TYPE_ECC_BUFFER); - if (curve->Gx == NULL || curve->Gy == NULL) + if (curve->Gx == NULL || curve->Gy == NULL) { + XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); ret = MEMORY_E; + } } if (ret == 0) { XMEMCPY((char*)curve->Gx, point + 2, curve->size * 2); @@ -11161,8 +11174,8 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, ((char*)curve->Gx)[curve->size * 2] = '\0'; ((char*)curve->Gy)[curve->size * 2] = '\0'; XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); - ret = ASNToHexString(input, inOutIdx, (char**)&curve->order, inSz, - key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->order, + inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { curve->cofactor = GetInteger7Bit(input, inOutIdx, inSz); From e6c95a08540a59070da39e98553da873aa2eccc0 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 26 Feb 2018 14:41:00 -0700 Subject: [PATCH 3/5] sanity check on input size --- wolfcrypt/src/asn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 2b2d7ad7b..11ef5b864 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11143,7 +11143,7 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - if (input[*inOutIdx] == ASN_BIT_STRING) { + if (*inOutIdx < inSz && input[*inOutIdx] == ASN_BIT_STRING) { len = 0; ret = GetASNHeader(input, ASN_BIT_STRING, inOutIdx, &len, inSz); *inOutIdx += len; From 00b6419964f742aa117df614421eff3232ffadca Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 26 Feb 2018 16:52:09 -0700 Subject: [PATCH 4/5] use XSTRLEN and revert adding outLen parameter --- wolfcrypt/src/asn.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 11ef5b864..f1313808b 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11032,19 +11032,14 @@ static void ByteToHex(byte n, char* str) str[1] = hexChar[n & 0xf]; } -/* - * outLen gets set to the size of buffer malloc'd (including null terminator) - * - * returns 0 on success - */ -static int ASNToHexString(const byte* input, word32* inOutIdx, int* outLen, - char** out, word32 inSz, void* heap, int heapType) +/* returns 0 on success */ +static int ASNToHexString(const byte* input, word32* inOutIdx, char** out, + word32 inSz, void* heap, int heapType) { int len; int i; char* str; - *outLen = 0; if (*inOutIdx >= inSz) { return BUFFER_E; } @@ -11064,8 +11059,7 @@ static int ASNToHexString(const byte* input, word32* inOutIdx, int* outLen, str[len*2] = '\0'; *inOutIdx += len; - *out = str; - *outLen = len * 2 + 1; + *out = str; return 0; } @@ -11125,8 +11119,8 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, } if (ret == 0) { SkipObjectId(input, inOutIdx, inSz); - ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->prime, - inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->prime, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { curve->size = (int)XSTRLEN(curve->prime) / 2; @@ -11135,12 +11129,12 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, ret = ASN_PARSE_E; } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->Af, - inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->Af, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->Bf, - inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->Bf, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { if (*inOutIdx < inSz && input[*inOutIdx] == ASN_BIT_STRING) { @@ -11150,9 +11144,9 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, } } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, &len, (char**)&point, inSz, + ret = ASNToHexString(input, inOutIdx, (char**)&point, inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); - if (ret == 0 && len < (curve->size * 4) + 2) { + if (ret == 0 && (int)XSTRLEN(point) < (curve->size * 4) + 2) { XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); ret = BUFFER_E; } @@ -11174,8 +11168,8 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, ((char*)curve->Gx)[curve->size * 2] = '\0'; ((char*)curve->Gy)[curve->size * 2] = '\0'; XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); - ret = ASNToHexString(input, inOutIdx, &len, (char**)&curve->order, - inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->order, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { curve->cofactor = GetInteger7Bit(input, inOutIdx, inSz); From 25e7dbd17abde73a69ec7b16f85683ed16ffb003 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Tue, 27 Feb 2018 23:30:50 -0700 Subject: [PATCH 5/5] add comment on sanity check --- wolfcrypt/src/asn.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index f1313808b..0447e743d 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11146,6 +11146,10 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, if (ret == 0) { ret = ASNToHexString(input, inOutIdx, (char**)&point, inSz, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + + /* sanity check that point buffer is not smaller than the expected + * size to hold ( 0 4 || Gx || Gy ) + * where Gx and Gy are each the size of curve->size * 2 */ if (ret == 0 && (int)XSTRLEN(point) < (curve->size * 4) + 2) { XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); ret = BUFFER_E;