1
0
mirror of https://github.com/trezor/trezor-firmware.git synced 2024-12-27 08:38:07 +00:00

Fixed undefined behavior

This fixes a shift by 32 and shifts on signed integer that overflow.
This commit is contained in:
Jochen Hoenicke 2018-03-22 20:36:22 +01:00 committed by Pavol Rusnak
parent 49272f8d86
commit 009850f6c9
6 changed files with 23 additions and 23 deletions

View File

@ -45,12 +45,12 @@ bool address_check_prefix(const uint8_t *addr, uint32_t address_type)
return address_type == (uint32_t)(addr[0]); return address_type == (uint32_t)(addr[0]);
} }
if (address_type <= 0xFFFF) { if (address_type <= 0xFFFF) {
return address_type == ((uint32_t)(addr[0] << 8) | (uint32_t)(addr[1])); return address_type == (((uint32_t) addr[0] << 8) | ((uint32_t) addr[1]));
} }
if (address_type <= 0xFFFFFF) { if (address_type <= 0xFFFFFF) {
return address_type == ((uint32_t)(addr[0] << 16) | (uint32_t)(addr[1] << 8) | (uint32_t)(addr[2])); return address_type == (((uint32_t) addr[0] << 16) | ((uint32_t) addr[1] << 8) | ((uint32_t) addr[2]));
} }
return address_type == ((uint32_t)(addr[0] << 24) | (uint32_t)(addr[1] << 16) | (uint32_t)(addr[2] << 8) | (uint32_t)(addr[3])); return address_type == (((uint32_t) addr[0] << 24) | ((uint32_t) addr[1] << 16) | ((uint32_t) addr[2] << 8) | ((uint32_t) addr[3]));
} }
#if USE_ETHEREUM #if USE_ETHEREUM

View File

@ -111,13 +111,13 @@ void bn_read_be(const uint8_t *in_number, bignum256 *out_number)
void bn_write_be(const bignum256 *in_number, uint8_t *out_number) void bn_write_be(const bignum256 *in_number, uint8_t *out_number)
{ {
int i; int i;
uint32_t temp = in_number->val[8] << 16; uint32_t temp = in_number->val[8];
for (i = 0; i < 8; i++) { for (i = 0; i < 8; i++) {
// invariant: temp = (in_number >> 30*(8-i)) << (16 + 2i) // invariant: temp = (in_number >> 30*(8-i))
uint32_t limb = in_number->val[7 - i]; uint32_t limb = in_number->val[7 - i];
temp |= limb >> (14 - 2*i); temp = (temp << (16 + 2*i)) | (limb >> (14 - 2*i));
write_be(out_number + i * 4, temp); write_be(out_number + i * 4, temp);
temp = limb << (18 + 2*i); temp = limb;
} }
} }
@ -146,13 +146,13 @@ void bn_read_le(const uint8_t *in_number, bignum256 *out_number)
void bn_write_le(const bignum256 *in_number, uint8_t *out_number) void bn_write_le(const bignum256 *in_number, uint8_t *out_number)
{ {
int i; int i;
uint32_t temp = in_number->val[8] << 16; uint32_t temp = in_number->val[8];
for (i = 0; i < 8; i++) { for (i = 0; i < 8; i++) {
// invariant: temp = (in_number >> 30*(8-i)) << (16 + 2i) // invariant: temp = (in_number >> 30*(8-i))
uint32_t limb = in_number->val[7 - i]; uint32_t limb = in_number->val[7 - i];
temp |= limb >> (14 - 2*i); temp = (temp << (16 + 2*i)) | (limb >> (14 - 2*i));
write_le(out_number + (7 - i) * 4, temp); write_le(out_number + (7 - i) * 4, temp);
temp = limb << (18 + 2*i); temp = limb;
} }
} }
@ -840,7 +840,7 @@ void bn_inverse(bignum256 *x, const bignum256 *prime)
// s + factor*prime mod 2^k == 0 // s + factor*prime mod 2^k == 0
// i.e. factor = s * -1/prime mod 2^k. // i.e. factor = s * -1/prime mod 2^k.
// Then compute s + factor*prime and shift right by k bits. // Then compute s + factor*prime and shift right by k bits.
uint32_t mask = (1 << k) - 1; uint32_t mask = (1u << k) - 1;
uint32_t factor = (inverse * us.a[8]) & mask; uint32_t factor = (inverse * us.a[8]) & mask;
temp = (us.a[8] + (uint64_t) pp[0] * factor) >> k; temp = (us.a[8] + (uint64_t) pp[0] * factor) >> k;
assert(((us.a[8] + pp[0] * factor) & mask) == 0); assert(((us.a[8] + pp[0] * factor) & mask) == 0);

View File

@ -168,7 +168,7 @@ uint32_t hdnode_fingerprint(HDNode *node)
hdnode_fill_public_key(node); hdnode_fill_public_key(node);
hasher_Raw(node->curve->hasher_type, node->public_key, 33, digest); hasher_Raw(node->curve->hasher_type, node->public_key, 33, digest);
ripemd160(digest, 32, digest); ripemd160(digest, 32, digest);
fingerprint = (digest[0] << 24) + (digest[1] << 16) + (digest[2] << 8) + digest[3]; fingerprint = ((uint32_t) digest[0] << 24) + (digest[1] << 16) + (digest[2] << 8) + digest[3];
memzero(digest, sizeof(digest)); memzero(digest, sizeof(digest));
return fingerprint; return fingerprint;
} }

16
nem.c
View File

@ -214,7 +214,7 @@ bool nem_transaction_create_transfer(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_TRANSFER, NEM_TRANSACTION_TYPE_TRANSFER,
network << 24 | (mosaics ? 2 : 1), (uint32_t) network << 24 | (mosaics ? 2 : 1),
timestamp, timestamp,
signer, signer,
fee, fee,
@ -292,7 +292,7 @@ bool nem_transaction_create_multisig(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_MULTISIG, NEM_TRANSACTION_TYPE_MULTISIG,
network << 24 | 1, (uint32_t) network << 24 | 1,
timestamp, timestamp,
signer, signer,
fee, fee,
@ -321,7 +321,7 @@ bool nem_transaction_create_multisig_signature(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_MULTISIG_SIGNATURE, NEM_TRANSACTION_TYPE_MULTISIG_SIGNATURE,
network << 24 | 1, (uint32_t) network << 24 | 1,
timestamp, timestamp,
signer, signer,
fee, fee,
@ -361,7 +361,7 @@ bool nem_transaction_create_provision_namespace(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_PROVISION_NAMESPACE, NEM_TRANSACTION_TYPE_PROVISION_NAMESPACE,
network << 24 | 1, (uint32_t) network << 24 | 1,
timestamp, timestamp,
signer, signer,
fee, fee,
@ -420,7 +420,7 @@ bool nem_transaction_create_mosaic_creation(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_MOSAIC_CREATION, NEM_TRANSACTION_TYPE_MOSAIC_CREATION,
network << 24 | 1, (uint32_t) network << 24 | 1,
timestamp, timestamp,
signer, signer,
fee, fee,
@ -506,7 +506,7 @@ bool nem_transaction_create_mosaic_supply_change(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_MOSAIC_SUPPLY_CHANGE, NEM_TRANSACTION_TYPE_MOSAIC_SUPPLY_CHANGE,
network << 24 | 1, (uint32_t) network << 24 | 1,
timestamp, timestamp,
signer, signer,
fee, fee,
@ -544,7 +544,7 @@ bool nem_transaction_create_aggregate_modification(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_AGGREGATE_MODIFICATION, NEM_TRANSACTION_TYPE_AGGREGATE_MODIFICATION,
network << 24 | (relative_change ? 2 : 1), (uint32_t) network << 24 | (relative_change ? 2 : 1),
timestamp, timestamp,
signer, signer,
fee, fee,
@ -600,7 +600,7 @@ bool nem_transaction_create_importance_transfer(nem_transaction_ctx *ctx,
bool ret = nem_transaction_write_common(ctx, bool ret = nem_transaction_write_common(ctx,
NEM_TRANSACTION_TYPE_IMPORTANCE_TRANSFER, NEM_TRANSACTION_TYPE_IMPORTANCE_TRANSFER,
network << 24 | 1, (uint32_t) network << 24 | 1,
timestamp, timestamp,
signer, signer,
fee, fee,

2
rand.c
View File

@ -45,7 +45,7 @@ uint32_t random32(void)
srand((unsigned)time(NULL)); srand((unsigned)time(NULL));
initialized = 1; initialized = 1;
} }
return ((rand() % 0xFF) | ((rand() % 0xFF) << 8) | ((rand() % 0xFF) << 16) | ((rand() % 0xFF) << 24)); return ((rand() & 0xFF) | ((rand() & 0xFF) << 8) | ((rand() & 0xFF) << 16) | ((uint32_t) (rand() & 0xFF) << 24));
} }
#endif /* RAND_PLATFORM_INDEPENDENT */ #endif /* RAND_PLATFORM_INDEPENDENT */

View File

@ -3028,7 +3028,7 @@ static void test_codepoints_curve(const ecdsa_curve *curve) {
for (i = 0; i < 64; i++) { for (i = 0; i < 64; i++) {
for (j = 0; j < 8; j++) { for (j = 0; j < 8; j++) {
bn_zero(&a); bn_zero(&a);
a.val[(4*i)/30] = (2*j+1) << (4*i % 30); a.val[(4*i)/30] = (uint32_t) (2*j+1) << (4*i % 30);
bn_normalize(&a); bn_normalize(&a);
// note that this is not a trivial test. We add 64 curve // note that this is not a trivial test. We add 64 curve
// points in the table to get that particular curve point. // points in the table to get that particular curve point.