Merge pull request #149 from Zondax/dev

Fixes from audit
Zondax · Jan 25, 2024 · fba709d · fba709d
2 parents 340c817 + 45aa435
commit fba709d
Show file tree

Hide file tree

Showing 23 changed files with 15,077 additions and 7,561 deletions.
diff --git a/app/Makefile.version b/app/Makefile.version
@@ -3,4 +3,4 @@ APPVERSION_M=0
 # This is the minor version of this release
 APPVERSION_N=23
 # This is the patch version of this release
-APPVERSION_P=12
+APPVERSION_P=13
diff --git a/app/src/base32.c b/app/src/base32.c
@@ -31,27 +31,26 @@ uint32_t base32_encode(const uint8_t *data,
         return 0;
     }
     uint32_t count = 0;
-    if (length > 0) {
-        uint32_t buffer = data[0];
-        uint32_t next = 1;
-        uint32_t bitsLeft = 8;
-        while (count < resultLen && (bitsLeft > 0 || next < length)) {
-            if (bitsLeft < 5) {
-                if (next < length) {
-                    buffer <<= 8;
-                    buffer |= data[next++] & 0xFF;
-                    bitsLeft += 8;
-                } else {
-                    uint32_t pad = 5u - bitsLeft;
-                    buffer <<= pad;
-                    bitsLeft += pad;
-                }
+    uint32_t buffer = data[0];
+    uint32_t next = 1;
+    uint32_t bitsLeft = 8;
+    while (count < resultLen && (bitsLeft > 0 || next < length)) {
+        if (bitsLeft < 5) {
+            if (next < length) {
+                buffer <<= 8;
+                buffer |= data[next++] & 0xFF;
+                bitsLeft += 8;
+            } else {
+                uint32_t pad = 5u - bitsLeft;
+                buffer <<= pad;
+                bitsLeft += pad;
             }
-            uint32_t index = 0x1Fu & (buffer >> (bitsLeft - 5u));
-            bitsLeft -= 5;
-            result[count++] = "abcdefghijklmnopqrstuvwxyz234567"[index];
         }
+        uint32_t index = 0x1Fu & (buffer >> (bitsLeft - 5u));
+        bitsLeft -= 5;
+        result[count++] = "abcdefghijklmnopqrstuvwxyz234567"[index];
     }
+
     if (count < resultLen) {
         result[count] = '\000';
     }

diff --git a/app/src/crypto_helper.c b/app/src/crypto_helper.c
@@ -128,7 +128,8 @@ uint16_t formatProtocol(const uint8_t *addressBytes,
 
     // f4 addresses contain actorID
     const uint16_t actorIdSize = (protocol == ADDRESS_PROTOCOL_DELEGATED) ? (addressSize - payloadSize - 1) : 0;
-    if (addressSize != payloadSize + 1 + actorIdSize) {
+    if (addressSize != payloadSize + 1 + actorIdSize
+        || payloadSize > ADDRESS_PROTOCOL_DELEGATED_MAX_SUBADDRESS_LEN) {
         return 0;
     }
     MEMCPY(payload_crc, addressBytes + 1 + actorIdSize, payloadSize);

diff --git a/app/src/eth_erc20.c b/app/src/eth_erc20.c
@@ -1,5 +1,5 @@
 /*******************************************************************************
-*  (c) 2018 - 2023 Zondax AG
+*  (c) 2018 - 2024 Zondax AG
 *
 *  Licensed under the Apache License, Version 2.0 (the "License");
 *  you may not use this file except in compliance with the License.
@@ -24,30 +24,45 @@ const uint8_t ERC20_TRANSFER_PREFIX[] = {0xa9, 0x05, 0x9c, 0xbb};
 #define DECIMAL_BASE 10
 const erc20_tokens_t supportedTokens[] = {
 
+    {{0x60, 0xE1, 0x77, 0x36, 0x36, 0xCF, 0x5E, 0x4A, 0x22, 0x7d, 0x9A, 0xC2, 0x4F, 0x20, 0xfE, 0xca, 0x03, 0x4e, 0xe2, 0x5A},
+    "WFIL ",
+    18},
 
     {{0x3C, 0x35, 0x01, 0xE6, 0xC3, 0x53, 0xDb, 0xaE, 0xDD, 0xFA, 0x90, 0x37, 0x69, 0x75, 0xCe, 0x7a, 0xCe, 0x4A, 0xc7, 0xa8},
     "stFIL ",
     18},
 
-    {{0x60, 0xE1, 0x77, 0x36, 0x36, 0xCF, 0x5E, 0x4A, 0x22, 0x7d, 0x9A, 0xC2, 0x4F, 0x20, 0xfE, 0xca, 0x03, 0x4e, 0xe2, 0x5A},
-    "WFIL ",
+    {{0x6A, 0x3F, 0x21, 0xd2, 0xA9, 0x2a, 0x15, 0x75, 0x29, 0x12, 0x97, 0x4B, 0xbB, 0xD5, 0xb1, 0x46, 0x9A, 0x72, 0xB2, 0x61},
+    "wstFIL ",
     18},
 
     {{0x69, 0x09, 0x08, 0xf7, 0xfa, 0x93, 0xaf, 0xC0, 0x40, 0xCF, 0xbD, 0x9f, 0xE1, 0xdD, 0xd2, 0xC2, 0x66, 0x8A, 0xa0, 0xe0},
     "iFIL ",
     18},
 
-    {{0x6A, 0x3F, 0x21, 0xd2, 0xA9, 0x2a, 0x15, 0x75, 0x29, 0x12, 0x97, 0x4B, 0xbB, 0xD5, 0xb1, 0x46, 0x9A, 0x72, 0xB2, 0x61},
-    "wstFIL ",
-    18},
-
     {{0xd0, 0x43, 0x77, 0x65, 0xD1, 0xDc, 0x0e, 0x2f, 0xA1, 0x4E, 0x97, 0xd2, 0x90, 0xF1, 0x35, 0xeF, 0xdF, 0x1a, 0x8a, 0x9A},
     "clFIL ",
     18},
 
-    {{0x42, 0x28, 0x49, 0xB3, 0x55, 0x03, 0x9b, 0xC5, 0x8F, 0x27, 0x80, 0xcc, 0x48, 0x54, 0x91, 0x9f, 0xC9, 0xcf, 0xaF, 0x94},
-    "USDT ",
+    {{0xeb, 0x46, 0x63, 0x42, 0xc4, 0xd4, 0x49, 0xbc, 0x9f, 0x53, 0xa8, 0x65, 0xd5, 0xcb, 0x90, 0x58, 0x6f, 0x40, 0x52, 0x15},
+    "axlUSDC ",
     6},
+
+    {{0xaa, 0xa9, 0x3a, 0xc7, 0x2b, 0xec, 0xfb, 0xbc, 0x91, 0x49, 0xf2, 0x93, 0x46, 0x6b, 0xbd, 0xaa, 0x4b, 0x5e, 0xf6, 0x8c},
+    "pFIL ",
+    18},
+
+    {{0x57, 0xe3, 0xbb, 0x9f, 0x79, 0x01, 0x85, 0xcf, 0xe7, 0x0c, 0xc2, 0xc1, 0x5e, 0xd5, 0xd6, 0xb8, 0x4d, 0xcf, 0x4a, 0xdb},
+    "wpFIL ",
+    18},
+
+    {{0xC5, 0xeA, 0x96, 0xDd, 0x36, 0x59, 0x83, 0xcf, 0xEc, 0x90, 0xE7, 0x2b, 0x6A, 0x2d, 0xaC, 0x95, 0x62, 0xf4, 0x58, 0xBa},
+    "SFT ",
+    18},
+
+    {{0x84, 0xb0, 0x38, 0xdb, 0x0f, 0xcd, 0xe4, 0xfa, 0xe5, 0x28, 0x10, 0x86, 0x03, 0xc7, 0x37, 0x66, 0x95, 0xdc, 0x21, 0x7f},
+    "NFIL ",
+    18},
 };
 
 parser_error_t getERC20Token(const rlp_t *data, char tokenSymbol[MAX_SYMBOL_LEN], uint8_t *decimals) {
@@ -118,7 +133,7 @@ parser_error_t printERC20Value(const rlp_t *data, char *outVal, uint16_t outValL
 
 bool validateERC20(rlp_t data) {
     // Check that data start with ERC20 prefix
-    if (data.rlpLen != ERC20_DATA_LENGTH || memcmp(data.ptr, ERC20_TRANSFER_PREFIX, 4) != 0) {
+    if (data.ptr == NULL || data.rlpLen != ERC20_DATA_LENGTH || memcmp(data.ptr, ERC20_TRANSFER_PREFIX, 4) != 0) {
         return false;
     }
 

diff --git a/app/src/eth_utils.c b/app/src/eth_utils.c
@@ -154,7 +154,7 @@ parser_error_t printRLPNumber(const rlp_t *num, char* outVal, uint16_t outValLen
 
 parser_error_t printEVMAddress(const rlp_t *address, char* outVal, uint16_t outValLen,
                                uint8_t pageIdx, uint8_t *pageCount) {
-    if (address == NULL || outVal == NULL || pageCount == NULL || address->rlpLen != ETH_ADDR_LEN) {
+    if (address == NULL || address->ptr == NULL || outVal == NULL || pageCount == NULL || address->rlpLen != ETH_ADDR_LEN) {
         return parser_unexpected_error;
     }
 

diff --git a/app/src/fil_utils.c b/app/src/fil_utils.c
@@ -97,12 +97,13 @@ parser_error_t printAddress(const address_t *a, char *outVal,
   char outBuffer[84 + 16];
   MEMZERO(outBuffer, sizeof(outBuffer));
 
-  if (formatProtocol(a->buffer, a->len, (uint8_t *)outBuffer,
-                     sizeof(outBuffer)) == 0) {
+  const uint16_t addressLen = formatProtocol(a->buffer, a->len, (uint8_t *)outBuffer, sizeof(outBuffer));
+  if (addressLen == 0 || addressLen > sizeof(outBuffer)){
     return parser_invalid_address;
   }
 
-  pageString(outVal, outValLen, outBuffer, pageIdx, pageCount);
+
+  pageStringExt(outVal, outValLen, outBuffer, addressLen, pageIdx, pageCount);
   return parser_ok;
 }
 
@@ -301,7 +302,7 @@ parser_error_t parse_check_prefix(CborValue *value, const char *prefix,
   bool is_equal = 0;
   CHECK_CBOR_MAP_ERR(cbor_value_copy_text_string(value, copied, &len, NULL)) {
     char l[100] = {0};
-    snprintf(l, 100, "len: %d", len);
+    snprintf(l, 100, "len: %d", (uint16_t)len);
     zemu_log_stack(l);
   }
   if (len != prefix_len)

diff --git a/app/src/parser_impl.c b/app/src/parser_impl.c
@@ -143,15 +143,14 @@ parser_error_t printValue(const struct CborValue *value, char *outVal,
     CborTag tag;
     CHECK_CBOR_MAP_ERR(cbor_value_get_tag(value, &tag))
     if (tag == TAG_CID) {
-      CHECK_CBOR_MAP_ERR(
-          cbor_value_copy_byte_string(value, buff, &buffLen, NULL /* next */))
+      CHECK_CBOR_MAP_ERR(cbor_value_copy_tag(value, buff, &buffLen, NULL /* next */))
       CHECK_APP_CANARY()
-      CHECK_PARSER_ERR(renderByteString(buff, buffLen, outVal, outValLen,
-                                        pageIdx, pageCount))
+      CHECK_PARSER_ERR(renderByteString(buff, buffLen, outVal, outValLen, pageIdx, pageCount))
       break;
     }
     return parser_unexpected_type;
   }
+
   default:
     snprintf(outVal, outValLen, "Type: %d", value->type);
   }
@@ -194,8 +193,7 @@ parser_error_t _printParam(const fil_base_tx_t *tx, uint8_t paramIdx,
                            parser_unexpected_type)
     // Not every ByteStringType must be interpreted as address. Depends on
     // method number and actor.
-    CHECK_PARSER_ERR(
-        printAddress(&tmpAddr, outVal, outValLen, pageIdx, pageCount));
+    CHECK_PARSER_ERR(printAddress(&tmpAddr, outVal, outValLen, pageIdx, pageCount));
     break;
   }
   case CborMapType:

diff --git a/app/src/parser_impl_eth.c b/app/src/parser_impl_eth.c
@@ -131,7 +131,7 @@ static parser_error_t readTxnType(parser_context_t *ctx, eth_tx_type_e *type) {
 }
 
 parser_error_t _readEth(parser_context_t *ctx, eth_tx_t *tx_obj) {
-
+    MEMZERO(&eth_tx_obj, sizeof(eth_tx_obj));
     CHECK_PARSER_ERR(readTxnType(ctx, &tx_obj->tx_type))
     // We expect a list with all the fields from the transaction
     rlp_t list = {0};

diff --git a/app/src/parser_impl_eth.h b/app/src/parser_impl_eth.h
@@ -39,7 +39,6 @@ parser_error_t _getNumItemsEth(uint8_t *numItems);
 
 parser_error_t _validateTxEth();
 
-// parser_error_t _computeV(unsigned int info, uint8_t *v);
 parser_error_t _computeV(parser_context_t *ctx, eth_tx_t *tx_obj, unsigned int info, uint8_t *v);
 
 #ifdef __cplusplus

diff --git a/app/src/uint256.c b/app/src/uint256.c
@@ -514,6 +514,10 @@ static void reverseString(char *str, uint32_t length) {
 
 bool tostring128(uint128_t *number, uint32_t baseParam, char *out,
                  uint32_t outLength) {
+    if (number == NULL || out == NULL || outLength == 0) {
+        return false;
+    }
+
     uint128_t rDiv;
     uint128_t rMod;
     uint128_t base;
@@ -532,13 +536,20 @@ bool tostring128(uint128_t *number, uint32_t baseParam, char *out,
         divmod128(&rDiv, &base, &rDiv, &rMod);
         out[offset++] = HEXDIGITS[(uint8_t)LOWER(rMod)];
     } while (!zero128(&rDiv));
+    if (offset >= outLength) {
+        return false;
+    }
     out[offset] = '\0';
     reverseString(out, offset);
     return true;
 }
 
 bool tostring256(uint256_t *number, uint32_t baseParam, char *out,
                  uint32_t outLength) {
+    if (number == NULL || out == NULL || outLength <= 1) {
+        return false;
+    }
+
     uint256_t rDiv;
     uint256_t rMod;
     uint256_t base;

diff --git a/deps/tinycbor/src/cbor.h b/deps/tinycbor/src/cbor.h
@@ -443,6 +443,13 @@ CBOR_INLINE_API CborError cbor_value_copy_byte_string(const CborValue *value, ui
     return _cbor_value_copy_string(value, buffer, buflen, next);
 }
 
+CBOR_INLINE_API CborError cbor_value_copy_tag(const CborValue *value, uint8_t *buffer,
+                                                      size_t *buflen, CborValue *next)
+{
+    assert(cbor_value_is_tag(value));
+    return _cbor_value_copy_string(value, buffer, buflen, next);
+}
+
 CBOR_INLINE_API CborError cbor_value_dup_text_string(const CborValue *value, char **buffer,
                                                      size_t *buflen, CborValue *next)
 {

diff --git a/deps/tinycbor/src/cborparser.c b/deps/tinycbor/src/cborparser.c
@@ -1105,7 +1105,7 @@ static CborError iterate_string_chunks(const CborValue *value, char *buffer, siz
     size_t total = 0;
     const void *ptr;
 
-    cbor_assert(cbor_value_is_byte_string(value) || cbor_value_is_text_string(value));
+    cbor_assert(cbor_value_is_byte_string(value) || cbor_value_is_text_string(value) || cbor_value_is_tag(value));
     if (!next)
         next = &tmp;
     *next = *value;