Merge pull request #100 from Zondax/fixes

add some safety checks

app/Makefile.version

@@ -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

app/src/crypto.c

@@ -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;
@@ -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};
@@ -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;
     }
 
@@ -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};
 
@@ -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};
@@ -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))

app/src/crypto_helper.c

@@ -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) {
@@ -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;
     }
 

app/src/substrate/substrate_coin.h

@@ -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
 

app/src/substrate/substrate_types.c

@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
 }
@@ -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;
@@ -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;
 }
@@ -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;
 }
@@ -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;
@@ -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;
 }
@@ -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;
 }
@@ -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;
 }
@@ -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;
     }
@@ -3111,7 +3127,7 @@ parser_error_t _toStringCondition(
         *pageCount += pages[i];
     }
 
-    if (pageIdx > *pageCount) {
+    if (pageIdx >= *pageCount) {
         return parser_display_idx_out_of_range;
     }
 
@@ -4419,7 +4435,7 @@ parser_error_t _toStringCall(
 
     pageIdx--;
 
-    if (pageIdx > *pageCount) {
+    if (pageIdx >= *pageCount) {
         return parser_display_idx_out_of_range;
     }
 

tests_zemu/package.json

@@ -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",