Merge pull request #132 from Zondax/fixes

Fixes

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=3
+APPVERSION_P=4

app/src/base32.c

@@ -26,28 +26,22 @@ uint32_t base32_encode(const uint8_t *data,
                        char *result,
                        uint32_t resultLen)
 {
-    if (length < 0 || length > (1 << 28))
-    {
-        return -1;
+    if (data == NULL || result == NULL ||
+        length > (1 << 28) || length == 0 || resultLen == 0) {
+        return 0;
     }
     uint32_t count = 0;
-    if (length > 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)
-                {
+        while (count < resultLen && (bitsLeft > 0 || next < length)) {
+            if (bitsLeft < 5) {
+                if (next < length) {
                     buffer <<= 8;
                     buffer |= data[next++] & 0xFF;
                     bitsLeft += 8;
-                }
-                else
-                {
+                } else {
                     uint32_t pad = 5u - bitsLeft;
                     buffer <<= pad;
                     bitsLeft += pad;
@@ -58,8 +52,7 @@ uint32_t base32_encode(const uint8_t *data,
             result[count++] = "abcdefghijklmnopqrstuvwxyz234567"[index];
         }
     }
-    if (count < resultLen)
-    {
+    if (count < resultLen) {
         result[count] = '\000';
     }
     return count;

app/src/common/actions.h

@@ -31,7 +31,7 @@ __Z_INLINE void app_sign() {
     uint16_t replyLen = 0;
 
     MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
-    zxerr_t err = zxerr_ok;
+    zxerr_t err = zxerr_unknown;
 
     // data digest for raw_bytes is computed differently, so it
     // requires its own signing method

app/src/common/tx.c

@@ -74,7 +74,7 @@ void tx_context_raw_bytes() {
   ctx_parsed_tx.tx_type = raw_bytes;
 }
 
-uint8_t tx_is_rawbytes() {
+bool tx_is_rawbytes() {
     return ctx_parsed_tx.tx_type == raw_bytes;
 }
 
@@ -136,7 +136,7 @@ zxerr_t tx_getItem(int8_t displayIdx,
 
     CHECK_ZXERR(tx_getNumItems(&numItems))
 
-    if (displayIdx < 0 || displayIdx > numItems) {
+    if (displayIdx < 0 || displayIdx >= numItems) {
         return zxerr_no_data;
     }
 

app/src/common/tx.h

@@ -26,7 +26,7 @@ void tx_context_client_deal();
 void tx_context_raw_bytes();
 
 // Signing is differently depending tx is rawBytes type
-uint8_t tx_is_rawbytes();
+bool tx_is_rawbytes();
 
 zxerr_t tx_rawbytes_init_state(uint8_t *buf, size_t buf_len);
 zxerr_t tx_rawbytes_update(uint8_t *buf, size_t buf_len);

app/src/crypto.c

@@ -329,7 +329,7 @@ zxerr_t crypto_fillAddress(uint8_t *buffer, uint16_t buffer_len, uint16_t *addrL
 }
 
 zxerr_t crypto_fillEthAddress(uint8_t *buffer, uint16_t buffer_len, uint16_t *addrLen) {
-    if (buffer == NULL || buffer_len < sizeof(answer_t) || addrLen == NULL) {
+    if (buffer == NULL || buffer_len < sizeof(answer_eth_t) || addrLen == NULL) {
         return zxerr_no_data;
     }
     MEMZERO(buffer, buffer_len);

app/src/crypto_helper.c

@@ -35,8 +35,8 @@ uint16_t decompressLEB128(const uint8_t *input, uint16_t inputSize, uint64_t *v)
     while (i < 10u && i < inputSize) {
         uint64_t b = input[i] & 0x7fu;
 
-        if (shift >= 63 && b > 1) {
-            // This will overflow uint64_t
+        if ((shift == 63 && b > 1) || (shift > 63 && b > 0)) {
+            // This will overflow uint64_t, break and return
             break;
         }
 
@@ -79,10 +79,10 @@ uint16_t formatProtocol(const uint8_t *addressBytes,
         case ADDRESS_PROTOCOL_ID: {
             uint64_t val = 0;
 
-            if (!decompressLEB128(addressBytes + 1, addressSize - 1, &val)) {
+            if (!decompressLEB128(addressBytes + 1, addressSize - 1, &val) ||
+                uint64_to_str((char *) formattedAddress + 2, formattedAddressSize - 2, val) != NULL) {
                 return 0;
             }
-            uint64_to_str((char *) formattedAddress + 2, formattedAddressSize - 2, val);
             return strnlen((const char *) formattedAddress, formattedAddressSize);
         }
         case ADDRESS_PROTOCOL_SECP256K1: {  // NOLINT(bugprone-branch-clone)
@@ -140,7 +140,7 @@ uint16_t formatProtocol(const uint8_t *addressBytes,
     if (base32_encode(payload_crc,
                       (uint32_t) (payloadSize + CHECKSUM_LENGTH),
                       (char *)(formattedAddress + offset),
-                      (uint32_t) (formattedAddressSize - offset)) < 0) {
+                      (uint32_t) (formattedAddressSize - offset)) == 0) {
         return 0;
     }
 

app/src/eth_utils.c

@@ -64,9 +64,9 @@ be_bytes_to_u64(const uint8_t *bytes, uint8_t len, uint64_t *num)
     }
 
     uint8_t *num_ptr = (uint8_t *)num;
-    for (int i = len; i--;) {
-        *num_ptr = bytes[i];
-        num_ptr += 1;
+    for (uint8_t i = 0; i < len; i++) {
+        *num_ptr = bytes[len - i - 1];
+        num_ptr++;
     }
 
     return 0;
@@ -146,30 +146,26 @@ parse_rlp_item(const uint8_t *data,
 
     uint8_t marker = data[0];
 
-
-    // first case item is just one byte
-    if ((marker - 0x00) * (marker - 0x7F) <= 0) {
+    if (marker <= 0x7F) {
+        // first case item is just one byte
         *read = 0;
         *item_len = 1;
         return rlp_ok;
-    }
 
-    // second case it is a sstring with a fixed length
-    if ((marker - 0x80) * (marker - 0xB7) <= 0) {
+    } else if (marker <= 0xB7) {
+        // second case it is a string with a fixed length
         uint8_t len = marker - 0x80;
-
         *read = 1;
         CHECK_RLP_LEN(dataLen, len + 1)
         *item_len = len;
         return rlp_ok;
-    }
 
-    // For strings longer than 55 bytes the length is encoded
-    // differently.
-    // The number of bytes that compose the length is encoded
-    // in the marker
-    // And then the length is just the number BE encoded
-    if ((marker - 0xB8) * (marker - 0xBF) <= 0) {
+    } else if (marker <= 0xBF) {
+        // For strings longer than 55 bytes the length is encoded
+        // differently.
+        // The number of bytes that compose the length is encoded
+        // in the marker
+        // And then the length is just the number BE encoded
         uint8_t num_bytes = marker - 0xB7;
         uint64_t len = 0;
         if (be_bytes_to_u64(&data[1], num_bytes, &len) != 0)
@@ -180,10 +176,9 @@ parse_rlp_item(const uint8_t *data,
         *item_len = len;
 
         return rlp_ok;
-    }
 
-    // simple list
-    if ((marker - 0xC0) * (marker - 0xF7) <= 0) {
+    } else if (marker <= 0xF7) {
+        // simple list
         uint8_t len = marker - 0xC0;
 
         *read = 1;
@@ -192,24 +187,21 @@ parse_rlp_item(const uint8_t *data,
         return rlp_ok;
     }
 
+    // marker >= 0xF8
     // For lists longer than 55 bytes the length is encoded
     // differently.
     // The number of bytes that compose the length is encoded
     // in the marker
     // And then the length is just the number BE encoded
-    if (marker >= 0xF8) {
-        uint8_t num_bytes = marker - 0xF7;
-        uint64_t len = 0;
-        if (be_bytes_to_u64(&data[1], num_bytes, &len) != 0)
-            return rlp_invalid_data;
-
-        CHECK_RLP_LEN(dataLen, len + 1 + num_bytes)
+    uint8_t num_bytes = marker - 0xF7;
+    uint64_t len = 0;
+    if (be_bytes_to_u64(&data[1], num_bytes, &len) != 0)
+        return rlp_invalid_data;
 
-        *read = 1 + num_bytes;
-        *item_len = len;
+    CHECK_RLP_LEN(dataLen, len + 1 + num_bytes)
 
-        return rlp_ok;
-    }
+    *read = 1 + num_bytes;
+    *item_len = len;
 
-    return rlp_invalid_data;
+    return rlp_ok;
 }

app/src/fil_utils.c

@@ -204,11 +204,11 @@ parser_error_t parse_cid(cid_t *cid, CborValue *value) {
         uint64_t version;
         uint64_t codec;
 
-        uint8_t base_offset = parse_varint(tmp,  cid_len, &base);
+        uint8_t base_offset = decompressLEB128(tmp,  cid_len, &base);
         bytes_read += base_offset;
 
-        bytes_read += parse_varint(tmp + bytes_read, cid_len - bytes_read, &version);
-        parse_varint(tmp + bytes_read, cid_len - bytes_read, &codec);
+        bytes_read += decompressLEB128(tmp + bytes_read, cid_len - bytes_read, &version);
+        decompressLEB128(tmp + bytes_read, cid_len - bytes_read, &codec);
 
         if ((uint8_t)codec != CID_CODEC || (uint8_t)version != CID_VERSION || (uint8_t)base != CID_BASE)
             return parser_invalid_cid;
@@ -236,41 +236,10 @@ parser_error_t printCid(cid_t *cid, char *outVal, uint16_t outValLen,
     // filecoin uses base32 which base prefix is b.
     *outBuffer = 'b';
 
-    size_t encoded_len = base32_encode(cid->str, cid->len, outBuffer + 1, sizeof(outBuffer)- 1);
-
-    if (encoded_len == 0)
+    if (base32_encode(cid->str, cid->len, outBuffer + 1, sizeof(outBuffer)- 1) == 0) {
         return parser_no_data;
+    }
 
     pageString(outVal, outValLen, outBuffer, pageIdx, pageCount);
-
     return parser_ok;
 }
-
-/**
-* reads a varint from buf.
-* result is written into /value
-* returns the amount of bytes read from buf
-* */
-size_t parse_varint(uint8_t *buf, size_t buf_len, uint64_t *value) {
-
-    uint8_t shift = 0;
-    uint8_t b;
-    size_t i;
-
-    if (value == NULL)
-        return 0;
-
-    *value = 0;
-
-    for (i = 0; i < buf_len; i++) {
-        b = buf[i];
-        *value |= (uint64_t)(b & 0x7F) << shift;
-        shift += 7;
-        if ((b & 0x80) == 0) {
-            break;
-        }
-    }
-
-    return i + 1;
-}
-

app/src/fil_utils.h

@@ -81,8 +81,6 @@ parser_error_t renderByteString(uint8_t *in, uint16_t inLen,
 parser_error_t parse_cid(cid_t *cid, CborValue *value);
 
 
-size_t parse_varint(uint8_t *buf, size_t buf_len, uint64_t *value);
-
 #ifdef __cplusplus
 }
 #endif