Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add some safety checks #100

Merged
merged 4 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/Makefile.version
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ APPVERSION_M=4
# This is the `spec_version` field of `Runtime`
APPVERSION_N=6000000
# This is the patch version of this release
APPVERSION_P=0
APPVERSION_P=1
17 changes: 10 additions & 7 deletions app/src/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ uint32_t hdPath[HDPATH_LEN_DEFAULT];

static zxerr_t crypto_extractPublicKey(key_kind_e addressKind, uint8_t *pubKey, uint16_t pubKeyLen) {
if (pubKey == NULL || pubKeyLen < PK_LEN_25519) {
return zxerr_invalid_crypto_settings;
return zxerr_buffer_too_small;
}

zxerr_t error = zxerr_unknown;
Expand Down Expand Up @@ -86,8 +86,8 @@ static zxerr_t crypto_extractPublicKey(key_kind_e addressKind, uint8_t *pubKey,
}

zxerr_t crypto_sign_ed25519(uint8_t *signature, uint16_t signatureMaxlen, const uint8_t *message, uint16_t messageLen) {
if (signature == NULL || message == NULL || signatureMaxlen < SIG_PLUS_TYPE_LEN) {
return zxerr_unknown;
if (signature == NULL || message == NULL || signatureMaxlen < SIG_PLUS_TYPE_LEN || messageLen == 0) {
return zxerr_buffer_too_small;
}
cx_ecfp_private_key_t cx_privateKey;
uint8_t privateKeyData[SK_LEN_25519] = {0};
Expand Down Expand Up @@ -147,7 +147,7 @@ void zeroize_sr25519_signdata(void) {
}

zxerr_t copy_sr25519_signdata(uint8_t *buffer, uint16_t bufferLen) {
if (SIG_PLUS_TYPE_LEN > bufferLen) {
if (buffer == NULL || SIG_PLUS_TYPE_LEN > bufferLen) {
return zxerr_buffer_too_small;
}

Expand All @@ -156,6 +156,9 @@ zxerr_t copy_sr25519_signdata(uint8_t *buffer, uint16_t bufferLen) {
}

static zxerr_t crypto_sign_sr25519_helper(const uint8_t *data, size_t len) {
if (data == NULL || len == 0) {
return zxerr_buffer_too_small;
}
uint8_t privateKeyData[SK_LEN_25519] = {0};
uint8_t pubkey[PK_LEN_25519] = {0};

Expand Down Expand Up @@ -193,8 +196,8 @@ static zxerr_t crypto_sign_sr25519_helper(const uint8_t *data, size_t len) {
}

zxerr_t crypto_sign_sr25519(const uint8_t *message, size_t messageLen) {
if (message == NULL) {
return zxerr_unknown;
if (message == NULL || messageLen == 0) {
return zxerr_buffer_too_small;
}

uint8_t messageDigest[BLAKE2B_DIGEST_SIZE] = {0};
Expand All @@ -217,7 +220,7 @@ zxerr_t crypto_sign_sr25519(const uint8_t *message, size_t messageLen) {

zxerr_t crypto_fillAddress(key_kind_e addressKind, uint8_t *buffer, uint16_t bufferLen, uint16_t *addrResponseLen) {
if (bufferLen < PK_LEN_25519 + SS58_ADDRESS_MAX_LEN) {
return zxerr_unknown;
return zxerr_buffer_too_small;
}
MEMZERO(buffer, bufferLen);
CHECK_ZXERR(crypto_extractPublicKey(addressKind, buffer, bufferLen))
Expand Down
8 changes: 4 additions & 4 deletions app/src/crypto_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ uint16_t crypto_SS58EncodePubkey(uint8_t *buffer, uint16_t buffer_len,
}
MEMZERO(buffer, buffer_len);

uint8_t hash[64];
uint8_t unencoded[36];
uint8_t hash[64] = {0};
uint8_t unencoded[36] = {0};

const uint8_t prefixSize = crypto_SS58CalculatePrefix(addressType, unencoded);
if (prefixSize == 0) {
Expand All @@ -83,15 +83,15 @@ uint16_t crypto_SS58EncodePubkey(uint8_t *buffer, uint16_t buffer_len,

memcpy(unencoded + prefixSize, pubkey, 32); // account id
if (ss58hash((uint8_t *) unencoded, 32 + prefixSize, hash, 64) != CX_OK) {
MEMZERO(buffer, buffer_len);
MEMZERO(unencoded, sizeof(unencoded));
return 0;
}
unencoded[32 + prefixSize] = hash[0];
unencoded[33 + prefixSize] = hash[1];

size_t outLen = buffer_len;
if (encode_base58(unencoded, 34 + prefixSize, buffer, &outLen) != 0) {
MEMZERO(buffer, buffer_len);
MEMZERO(unencoded, sizeof(unencoded));
return 0;
}

Expand Down
2 changes: 1 addition & 1 deletion app/src/substrate/substrate_coin.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ typedef enum {
#define SUPPORTED_TX_VERSION_CURRENT LEDGER_MAJOR_VERSION
#define SUPPORTED_TX_VERSION_PREVIOUS (LEDGER_MAJOR_VERSION - 1)
#define SUPPORTED_SPEC_VERSION (LEDGER_MINOR_VERSION + 0)
#define SUPPORTED_MINIMUM_SPEC_VERSION 0
#define SUPPORTED_MINIMUM_SPEC_VERSION 6000000

#define COIN_AMOUNT_DECIMAL_PLACES 6

Expand Down
20 changes: 18 additions & 2 deletions app/src/substrate/substrate_types.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ parser_error_t _readClaim(parser_context_t* c, pd_Claim_t* v)

parser_error_t _readDispatchableNames(parser_context_t* c, pd_DispatchableNames_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt8(c, &v->value))
switch (v->value) {
case 0: // Whole
Expand Down Expand Up @@ -427,6 +428,7 @@ parser_error_t _readTax(parser_context_t* c, pd_Tax_t* v)

parser_error_t _readAssetPermissions(parser_context_t* c, pd_AssetPermissions_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt8(c, &v->value))
switch (v->value) {
case 0: // Whole
Expand Down Expand Up @@ -475,6 +477,7 @@ parser_error_t _readDocumentType(parser_context_t* c, pd_DocumentType_t* v)

parser_error_t _readExtrinsicPermissions(parser_context_t* c, pd_ExtrinsicPermissions_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt8(c, &v->value))
switch (v->value) {
case 0: // Whole
Expand Down Expand Up @@ -547,6 +550,7 @@ parser_error_t _readMultiSignature(parser_context_t* c, pd_MultiSignature_t* v)

parser_error_t _readPortfolioPermissions(parser_context_t* c, pd_PortfolioPermissions_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt8(c, &v->value))
switch (v->value) {
case 0: // Whole
Expand Down Expand Up @@ -801,6 +805,7 @@ parser_error_t _readPermissions(parser_context_t* c, pd_Permissions_t* v)

parser_error_t _readPipId(parser_context_t* c, pd_PipId_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->value))
return parser_ok;
}
Expand Down Expand Up @@ -916,6 +921,7 @@ parser_error_t _readBecomeAgent(parser_context_t* c, pd_BecomeAgent_t* v)

parser_error_t _readBeneficiary(parser_context_t* c, pd_Beneficiary_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readIdentityId(c, &v->identity))
CHECK_ERROR(_readBalance(c, &v->balance))
return parser_ok;
Expand Down Expand Up @@ -992,12 +998,14 @@ parser_error_t _readCreateChildIdentityWithAuthAccountId(parser_context_t* c, pd

parser_error_t _readCustomAssetTypeId(parser_context_t* c, pd_CustomAssetTypeId_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->value))
return parser_ok;
}

parser_error_t _readDocumentId(parser_context_t* c, pd_DocumentId_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->value))
return parser_ok;
}
Expand Down Expand Up @@ -1066,6 +1074,7 @@ parser_error_t _readLeg(parser_context_t* c, pd_Leg_t* v)

parser_error_t _readLocalCAId(parser_context_t* c, pd_LocalCAId_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->value))
return parser_ok;
}
Expand Down Expand Up @@ -1219,6 +1228,7 @@ parser_error_t _readTupleExtrinsicIdbool(parser_context_t* c, pd_TupleExtrinsicI

parser_error_t _readTupleIdentityIdbool(parser_context_t* c, pd_TupleIdentityIdbool_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readIdentityId(c, &v->identity))
CHECK_ERROR(_readBool(c, &v->val))
return parser_ok;
Expand Down Expand Up @@ -1424,6 +1434,7 @@ parser_error_t _readCAId(parser_context_t* c, pd_CAId_t* v)

parser_error_t _readCodeHash(parser_context_t* c, pd_CodeHash_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readHash(c, &v->hash))
return parser_ok;
}
Expand Down Expand Up @@ -1558,6 +1569,7 @@ parser_error_t _readVecCall(parser_context_t* c, pd_VecCall_t* v)

parser_error_t _readAGId(parser_context_t* c, pd_AGId_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->value))
return parser_ok;
}
Expand Down Expand Up @@ -1695,19 +1707,22 @@ parser_error_t _readPortfolioName(parser_context_t* c, pd_PortfolioName_t* v)

parser_error_t _readPosRatio(parser_context_t* c, pd_PosRatio_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->numerator))
CHECK_ERROR(_readUInt32(c, &v->denominator))
return parser_ok;
}

parser_error_t _readProposalIndex(parser_context_t* c, pd_ProposalIndex_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt32(c, &v->value))
return parser_ok;
}

parser_error_t _readSkippedCount(parser_context_t* c, pd_SkippedCount_t* v)
{
CHECK_INPUT()
CHECK_ERROR(_readUInt8(c, &v->value))
return parser_ok;
}
Expand Down Expand Up @@ -2986,6 +3001,7 @@ parser_error_t _toStringMemo(
uint8_t pageIdx,
uint8_t* pageCount)
{
CLEAN_AND_CHECK()
if (formatBufferData(v->_ptr, v->_len, outValue, outValueLen, pageIdx, pageCount) != zxerr_ok) {
return parser_print_not_supported;
}
Expand Down Expand Up @@ -3111,7 +3127,7 @@ parser_error_t _toStringCondition(
*pageCount += pages[i];
}

if (pageIdx > *pageCount) {
if (pageIdx >= *pageCount) {
return parser_display_idx_out_of_range;
}

Expand Down Expand Up @@ -4419,7 +4435,7 @@ parser_error_t _toStringCall(

pageIdx--;

if (pageIdx > *pageCount) {
if (pageIdx >= *pageCount) {
return parser_display_idx_out_of_range;
}

Expand Down
2 changes: 1 addition & 1 deletion deps/ledger-zxlib
2 changes: 1 addition & 1 deletion tests_zemu/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
},
"dependencies": {
"@zondax/ledger-substrate": "^0.41.1",
"@zondax/zemu": "^0.44.0"
"@zondax/zemu": "^0.44.2"
},
"devDependencies": {
"@types/jest": "^29.2.6",
Expand Down
Binary file modified tests_zemu/snapshots/s-mainmenu/00004.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests_zemu/snapshots/s-mainmenu/00010.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests_zemu/snapshots/sp-mainmenu/00004.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests_zemu/snapshots/sp-mainmenu/00010.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests_zemu/snapshots/st-mainmenu/00001.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests_zemu/snapshots/x-mainmenu/00004.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests_zemu/snapshots/x-mainmenu/00010.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading