From 13cb43ad4c9be48914f17ea434f858aba0f8a06e Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Sun, 24 Sep 2017 13:11:35 +0200 Subject: [PATCH 1/2] initialize 'flags' etc. to invalid values before trying to decode --- src/pk/dh/dh_import.c | 8 +++++++- src/pk/dsa/dsa_decrypt_key.c | 3 ++- src/pk/dsa/dsa_import.c | 9 +++++++-- src/pk/ecc/ecc_decrypt_key.c | 3 ++- src/pk/ecc/ecc_import.c | 7 ++++++- src/pk/rsa/rsa_import.c | 1 + 6 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/pk/dh/dh_import.c b/src/pk/dh/dh_import.c index 579a6aa..b600b5c 100644 --- a/src/pk/dh/dh_import.c +++ b/src/pk/dh/dh_import.c @@ -32,6 +32,8 @@ int dh_import(const unsigned char *in, unsigned long inlen, dh_key *key) return err; } + version = 666; + flags[0] = 0xff; /* find out what type of key it is */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_SHORT_INTEGER, 1UL, &version, @@ -58,7 +60,7 @@ int dh_import(const unsigned char *in, unsigned long inlen, dh_key *key) goto error; } } - else { + else if (flags[0] == 0) { key->type = PK_PUBLIC; if ((err = der_decode_sequence_multi(in, inlen, LTC_ASN1_SHORT_INTEGER, 1UL, &version, @@ -70,6 +72,10 @@ int dh_import(const unsigned char *in, unsigned long inlen, dh_key *key) goto error; } } + else { + err = CRYPT_INVALID_PACKET; + goto error; + } } else { err = CRYPT_INVALID_PACKET; diff --git a/src/pk/dsa/dsa_decrypt_key.c b/src/pk/dsa/dsa_decrypt_key.c index 806ef3e..67426b8 100644 --- a/src/pk/dsa/dsa_decrypt_key.c +++ b/src/pk/dsa/dsa_decrypt_key.c @@ -30,7 +30,8 @@ int dsa_decrypt_key(const unsigned char *in, unsigned long inlen, { unsigned char *skey, *expt; void *g_pub; - unsigned long x, y, hashOID[32]; + unsigned long x, y; + unsigned long hashOID[32] = { 0 }; int hash, err; ltc_asn1_list decode[3]; diff --git a/src/pk/dsa/dsa_import.c b/src/pk/dsa/dsa_import.c index 08d64b7..8d949eb 100644 --- a/src/pk/dsa/dsa_import.c +++ b/src/pk/dsa/dsa_import.c @@ -38,13 +38,14 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key) return CRYPT_MEM; } + flags[0] = 0xff; /* try to match the old libtomcrypt format */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_BIT_STRING, 1UL, flags, LTC_ASN1_EOL, 0UL, NULL); if (err == CRYPT_OK || err == CRYPT_PK_INVALID_SIZE) { /* private key */ - if (flags[0]) { + if (flags[0] == 1) { if ((err = der_decode_sequence_multi(in, inlen, LTC_ASN1_BIT_STRING, 1UL, flags, LTC_ASN1_INTEGER, 1UL, key->g, @@ -59,7 +60,7 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key) goto LBL_OK; } /* public key */ - else { + else if (flags[0] == 0) { if ((err = der_decode_sequence_multi(in, inlen, LTC_ASN1_BIT_STRING, 1UL, flags, LTC_ASN1_INTEGER, 1UL, key->g, @@ -72,6 +73,10 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key) key->type = PK_PUBLIC; goto LBL_OK; } + else { + err = CRYPT_INVALID_PACKET; + goto LBL_ERR; + } } /* get key type */ if ((err = der_decode_sequence_multi(in, inlen, diff --git a/src/pk/ecc/ecc_decrypt_key.c b/src/pk/ecc/ecc_decrypt_key.c index 4a16de9..9492401 100644 --- a/src/pk/ecc/ecc_decrypt_key.c +++ b/src/pk/ecc/ecc_decrypt_key.c @@ -35,7 +35,8 @@ int ecc_decrypt_key(const unsigned char *in, unsigned long inlen, ecc_key *key) { unsigned char *ecc_shared, *skey, *pub_expt; - unsigned long x, y, hashOID[32]; + unsigned long x, y; + unsigned long hashOID[32] = { 0 }; int hash, err; ecc_key pubkey; ltc_asn1_list decode[3]; diff --git a/src/pk/ecc/ecc_import.c b/src/pk/ecc/ecc_import.c index 034c9bd..fce70e2 100644 --- a/src/pk/ecc/ecc_import.c +++ b/src/pk/ecc/ecc_import.c @@ -104,6 +104,7 @@ int ecc_import_ex(const unsigned char *in, unsigned long inlen, ecc_key *key, co return CRYPT_MEM; } + flags[0] = 0xff; /* find out what type of key it is */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_BIT_STRING, 1UL, flags, LTC_ASN1_EOL, 0UL, NULL); @@ -124,7 +125,7 @@ int ecc_import_ex(const unsigned char *in, unsigned long inlen, ecc_key *key, co LTC_ASN1_EOL, 0UL, NULL)) != CRYPT_OK) { goto done; } - } else { + } else if (flags[0] == 0) { /* public key */ key->type = PK_PUBLIC; if ((err = der_decode_sequence_multi(in, inlen, @@ -136,6 +137,10 @@ int ecc_import_ex(const unsigned char *in, unsigned long inlen, ecc_key *key, co goto done; } } + else { + err = CRYPT_INVALID_PACKET; + goto done; + } if (dp == NULL) { /* find the idx */ diff --git a/src/pk/rsa/rsa_import.c b/src/pk/rsa/rsa_import.c index fbae39b..db432b5 100644 --- a/src/pk/rsa/rsa_import.c +++ b/src/pk/rsa/rsa_import.c @@ -65,6 +65,7 @@ int rsa_import(const unsigned char *in, unsigned long inlen, rsa_key *key) goto LBL_FREE; } + mp_set_int(key->N, 666); /* not SSL public key, try to match against PKCS #1 standards */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_INTEGER, 1UL, key->N, LTC_ASN1_EOL, 0UL, NULL); From 4a8bfc0a21da06d445e0c89c9014a6157ecb0645 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Mon, 25 Sep 2017 21:58:50 +0200 Subject: [PATCH 2/2] introduce CRYPT_INPUT_TOO_LONG --- src/headers/tomcrypt.h | 3 ++- src/misc/crypt/crypt_constants.c | 2 +- src/misc/error_to_string.c | 3 ++- src/pk/asn1/der/sequence/der_decode_sequence_ex.c | 2 +- src/pk/dh/dh_import.c | 4 +--- src/pk/dsa/dsa_decrypt_key.c | 2 +- src/pk/dsa/dsa_import.c | 3 +-- src/pk/ecc/ecc_decrypt_key.c | 2 +- src/pk/ecc/ecc_import.c | 3 +-- src/pk/rsa/rsa_import.c | 3 +-- 10 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/headers/tomcrypt.h b/src/headers/tomcrypt.h index 96b0e3a..e736a54 100644 --- a/src/headers/tomcrypt.h +++ b/src/headers/tomcrypt.h @@ -68,7 +68,8 @@ enum { CRYPT_OVERFLOW, /* An overflow of a value was detected/prevented */ CRYPT_UNUSED1, /* UNUSED1 */ - CRYPT_UNUSED2, /* UNUSED2 */ + + CRYPT_INPUT_TOO_LONG, /* The input was longer than expected. */ CRYPT_PK_INVALID_SIZE, /* Invalid size input for PK parameters */ diff --git a/src/misc/crypt/crypt_constants.c b/src/misc/crypt/crypt_constants.c index f866f3e..c63d3f8 100644 --- a/src/misc/crypt/crypt_constants.c +++ b/src/misc/crypt/crypt_constants.c @@ -48,7 +48,7 @@ static const crypt_constant _crypt_constants[] = { _C_STRINGIFY(CRYPT_PK_INVALID_TYPE), _C_STRINGIFY(CRYPT_OVERFLOW), _C_STRINGIFY(CRYPT_UNUSED1), - _C_STRINGIFY(CRYPT_UNUSED2), + _C_STRINGIFY(CRYPT_INPUT_TOO_LONG), _C_STRINGIFY(CRYPT_PK_INVALID_SIZE), _C_STRINGIFY(CRYPT_INVALID_PRIME_SIZE), _C_STRINGIFY(CRYPT_PK_INVALID_PADDING), diff --git a/src/misc/error_to_string.c b/src/misc/error_to_string.c index 2a0d3f8..707f835 100644 --- a/src/misc/error_to_string.c +++ b/src/misc/error_to_string.c @@ -47,7 +47,8 @@ static const char * const err_2_str[] = "An overflow of a value was detected/prevented.", "UNUSED1.", - "UNUSED2.", + + "The input was longer than expected.", "Invalid sized parameter.", diff --git a/src/pk/asn1/der/sequence/der_decode_sequence_ex.c b/src/pk/asn1/der/sequence/der_decode_sequence_ex.c index 8a6755e..b820c68 100644 --- a/src/pk/asn1/der/sequence/der_decode_sequence_ex.c +++ b/src/pk/asn1/der/sequence/der_decode_sequence_ex.c @@ -314,7 +314,7 @@ int der_decode_sequence_ex(const unsigned char *in, unsigned long inlen, if (inlen == 0) { err = CRYPT_OK; } else { - err = CRYPT_PK_INVALID_SIZE; + err = CRYPT_INPUT_TOO_LONG; } LBL_ERR: diff --git a/src/pk/dh/dh_import.c b/src/pk/dh/dh_import.c index b600b5c..601e5e7 100644 --- a/src/pk/dh/dh_import.c +++ b/src/pk/dh/dh_import.c @@ -32,14 +32,12 @@ int dh_import(const unsigned char *in, unsigned long inlen, dh_key *key) return err; } - version = 666; - flags[0] = 0xff; /* find out what type of key it is */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_SHORT_INTEGER, 1UL, &version, LTC_ASN1_BIT_STRING, 1UL, &flags, LTC_ASN1_EOL, 0UL, NULL); - if (err != CRYPT_OK && err != CRYPT_PK_INVALID_SIZE) { + if (err != CRYPT_OK && err != CRYPT_INPUT_TOO_LONG) { goto error; } diff --git a/src/pk/dsa/dsa_decrypt_key.c b/src/pk/dsa/dsa_decrypt_key.c index 67426b8..ef4e1dd 100644 --- a/src/pk/dsa/dsa_decrypt_key.c +++ b/src/pk/dsa/dsa_decrypt_key.c @@ -48,7 +48,7 @@ int dsa_decrypt_key(const unsigned char *in, unsigned long inlen, /* decode to find out hash */ LTC_SET_ASN1(decode, 0, LTC_ASN1_OBJECT_IDENTIFIER, hashOID, sizeof(hashOID)/sizeof(hashOID[0])); err = der_decode_sequence(in, inlen, decode, 1); - if (err != CRYPT_OK && err != CRYPT_PK_INVALID_SIZE) { + if (err != CRYPT_OK && err != CRYPT_INPUT_TOO_LONG) { return err; } diff --git a/src/pk/dsa/dsa_import.c b/src/pk/dsa/dsa_import.c index 8d949eb..f1f0633 100644 --- a/src/pk/dsa/dsa_import.c +++ b/src/pk/dsa/dsa_import.c @@ -38,12 +38,11 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key) return CRYPT_MEM; } - flags[0] = 0xff; /* try to match the old libtomcrypt format */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_BIT_STRING, 1UL, flags, LTC_ASN1_EOL, 0UL, NULL); - if (err == CRYPT_OK || err == CRYPT_PK_INVALID_SIZE) { + if (err == CRYPT_OK || err == CRYPT_INPUT_TOO_LONG) { /* private key */ if (flags[0] == 1) { if ((err = der_decode_sequence_multi(in, inlen, diff --git a/src/pk/ecc/ecc_decrypt_key.c b/src/pk/ecc/ecc_decrypt_key.c index 9492401..51b894b 100644 --- a/src/pk/ecc/ecc_decrypt_key.c +++ b/src/pk/ecc/ecc_decrypt_key.c @@ -54,7 +54,7 @@ int ecc_decrypt_key(const unsigned char *in, unsigned long inlen, /* decode to find out hash */ LTC_SET_ASN1(decode, 0, LTC_ASN1_OBJECT_IDENTIFIER, hashOID, sizeof(hashOID)/sizeof(hashOID[0])); err = der_decode_sequence(in, inlen, decode, 1); - if (err != CRYPT_OK && err != CRYPT_PK_INVALID_SIZE) { + if (err != CRYPT_OK && err != CRYPT_INPUT_TOO_LONG) { return err; } diff --git a/src/pk/ecc/ecc_import.c b/src/pk/ecc/ecc_import.c index fce70e2..c6d474f 100644 --- a/src/pk/ecc/ecc_import.c +++ b/src/pk/ecc/ecc_import.c @@ -104,11 +104,10 @@ int ecc_import_ex(const unsigned char *in, unsigned long inlen, ecc_key *key, co return CRYPT_MEM; } - flags[0] = 0xff; /* find out what type of key it is */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_BIT_STRING, 1UL, flags, LTC_ASN1_EOL, 0UL, NULL); - if (err != CRYPT_OK && err != CRYPT_PK_INVALID_SIZE) { + if (err != CRYPT_OK && err != CRYPT_INPUT_TOO_LONG) { goto done; } diff --git a/src/pk/rsa/rsa_import.c b/src/pk/rsa/rsa_import.c index db432b5..7140a73 100644 --- a/src/pk/rsa/rsa_import.c +++ b/src/pk/rsa/rsa_import.c @@ -65,12 +65,11 @@ int rsa_import(const unsigned char *in, unsigned long inlen, rsa_key *key) goto LBL_FREE; } - mp_set_int(key->N, 666); /* not SSL public key, try to match against PKCS #1 standards */ err = der_decode_sequence_multi(in, inlen, LTC_ASN1_INTEGER, 1UL, key->N, LTC_ASN1_EOL, 0UL, NULL); - if (err != CRYPT_OK && err != CRYPT_PK_INVALID_SIZE) { + if (err != CRYPT_OK && err != CRYPT_INPUT_TOO_LONG) { goto LBL_ERR; }