d-bis/app-ethereum
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Code improvements / Fix warnings

Browse Source 
- Fix CodeQL warnings
- Port lots of improvements/fixes from PR #225
- replace 'array_hexstr' and '%*H' format by sdk function 'format_hex'
- Add 'noreturn' attribute in 'main.c'
This commit is contained in:
Charles-Edouard de la Vergne
2024-04-12 16:40:05 +02:00
parent 90d536402b
commit aa38ee9959
29 changed files with 103 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
Makefile
View File

@@ -21,8 +21,6 @@ endif
include $(BOLOS_SDK)/Makefile.defines
# Allows to use sprintf(..., "0x%.*H", ...)
CFLAGS += -Wno-format-invalid-specifier -Wno-format-extra-args
########################################
# Mandatory configuration #
########################################
@@ -54,6 +52,7 @@ APP_SOURCE_FILES += ./ethereum-plugin-sdk/src/common_utils.c
APP_SOURCE_FILES += ./ethereum-plugin-sdk/src/plugin_utils.c
INCLUDES_PATH += ./ethereum-plugin-sdk/src
APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/crypto_helpers.c
APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/format.c
INCLUDES_PATH += ${BOLOS_SDK}/lib_standard_app
ifeq ($(TARGET_NAME),TARGET_STAX)

4
src/eth_plugin_handler.c
View File

@@ -102,11 +102,11 @@ static void eth_plugin_perform_init_default(uint8_t *contractAddress,
static bool eth_plugin_perform_init_old_internal(uint8_t *contractAddress,
ethPluginInitContract_t *init) {
uint8_t i, j;
const uint8_t **selectors;
const uint8_t *const *selectors;
// Search internal plugin list
for (i = 0;; i++) {
selectors = (const uint8_t **) PIC(INTERNAL_ETH_PLUGINS[i].selectors);
selectors = (const uint8_t *const *) PIC(INTERNAL_ETH_PLUGINS[i].selectors);
if (selectors == NULL) {
break;
}

4
src/eth_plugin_internal.c
View File

@@ -25,11 +25,11 @@ const uint8_t* const ETH2_SELECTORS[NUM_ETH2_SELECTORS] = {ETH2_DEPOSIT_SELECTOR
// All internal alias names start with 'minus'
const internalEthPlugin_t INTERNAL_ETH_PLUGINS[] = {
{NULL, (const uint8_t**) ERC20_SELECTORS, NUM_ERC20_SELECTORS, "-erc20", erc20_plugin_call},
{NULL, ERC20_SELECTORS, NUM_ERC20_SELECTORS, "-erc20", erc20_plugin_call},
#ifdef HAVE_ETH2
{NULL, (const uint8_t**) ETH2_SELECTORS, NUM_ETH2_SELECTORS, "-eth2", eth2_plugin_call},
{NULL, ETH2_SELECTORS, NUM_ETH2_SELECTORS, "-eth2", eth2_plugin_call},
#endif

4
src/eth_plugin_internal.h
View File

@@ -8,12 +8,12 @@
void erc721_plugin_call(int message, void* parameters);
void erc1155_plugin_call(int message, void* parameters);
typedef bool (*PluginAvailableCheck)(void);
typedef bool (*const PluginAvailableCheck)(void);
typedef void (*PluginCall)(int, void*);
typedef struct internalEthPlugin_t {
PluginAvailableCheck availableCheck;
const uint8_t** selectors;
const uint8_t* const* selectors;
uint8_t num_selectors;
char alias[10];
PluginCall impl;

2
src/handle_check_address.c
View File

@@ -7,7 +7,7 @@
#define ZERO(x) explicit_bzero(&x, sizeof(x))
void handle_check_address(check_address_parameters_t* params, chain_config_t* chain_config) {
void handle_check_address(check_address_parameters_t* params, const chain_config_t* chain_config) {
params->result = 0;
PRINTF("Params on the address %d\n", (unsigned int) params);
PRINTF("Address to check %s\n", params->address_to_check);

2
src/handle_check_address.h
View File

@@ -5,6 +5,6 @@
#include "chainConfig.h"
void handle_check_address(check_address_parameters_t* check_address_params,
chain_config_t* chain_config);
const chain_config_t* chain_config);
#endif // _HANDLE_CHECK_ADDRESS_H_

4
src/handle_swap_sign_transaction.c
View File

@@ -14,7 +14,7 @@
static uint8_t* G_swap_sign_return_value_address;
bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
chain_config_t* config) {
const chain_config_t* config) {
// first copy parameters to stack, and then to global data.
// We need this "trick" as the input data position can overlap with app-ethereum globals
txStringProperties_t stack_data;
@@ -80,7 +80,7 @@ void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_succes
os_lib_end();
}
void __attribute__((noreturn)) handle_swap_sign_transaction(chain_config_t* config) {
void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t* config) {
#ifdef HAVE_NBGL
// On Stax, display a spinner at startup
UX_INIT();

4
src/handle_swap_sign_transaction.h
View File

@@ -4,8 +4,8 @@
#include "chainConfig.h"
bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
chain_config_t* config);
const chain_config_t* config);
void __attribute__((noreturn)) handle_swap_sign_transaction(chain_config_t* config);
void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t* config);
void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_success);

12
src/main.c
View File

@@ -67,7 +67,7 @@ const internalStorage_t N_storage_real;
#ifdef HAVE_NBGL
caller_app_t *caller_app = NULL;
#endif
chain_config_t *chainConfig = NULL;
const chain_config_t *chainConfig;
void reset_app_context() {
// PRINTF("!!RESET_APP_CONTEXT\n");
@@ -114,7 +114,7 @@ unsigned short io_exchange_al(unsigned char channel, unsigned short tx_len) {
return 0;
}
extraInfo_t *getKnownToken(uint8_t *contractAddress) {
extraInfo_t *getKnownToken(const uint8_t *contractAddress) {
union extraInfo_t *currentItem = NULL;
// Works for ERC-20 & NFT tokens since both structs in the union have the
// contract address aligned
@@ -456,7 +456,7 @@ void app_main(void) {
// override point, but nothing more to do
#ifdef HAVE_BAGL
void io_seproxyhal_display(const bagl_element_t *element) {
io_seproxyhal_display_default((bagl_element_t *) element);
io_seproxyhal_display_default(element);
}
#endif
@@ -526,7 +526,7 @@ void init_coin_config(chain_config_t *coin_config) {
coin_config->chainId = CHAIN_ID;
}
void coin_main(libargs_t *args) {
__attribute__((noreturn)) void coin_main(libargs_t *args) {
chain_config_t config;
if (args) {
if (args->chain_config != NULL) {
@@ -612,10 +612,10 @@ void coin_main(libargs_t *args) {
}
END_TRY;
}
app_exit();
os_sched_exit(-1);
}
void library_main(libargs_t *args) {
__attribute__((noreturn)) void library_main(libargs_t *args) {
chain_config_t coin_config;
if (args->chain_config == NULL) {
// We have been started directly by Exchange, not by a Clone. Init default chain

2
src/shared_context.h
View File

@@ -147,7 +147,7 @@ typedef union {
strDataTmp_t tmp;
} strings_t;
extern chain_config_t *chainConfig;
extern const chain_config_t *chainConfig;
extern tmpCtx_t tmpCtx;
extern txContext_t txContext;

2
src/ui_callbacks.h
View File

@@ -26,6 +26,6 @@ void ui_warning_contract_data(void);
void io_seproxyhal_send_status(uint32_t sw);
void finalizeParsing(bool direct);
extraInfo_t *getKnownToken(uint8_t *contractAddress);
extraInfo_t *getKnownToken(const uint8_t *contractAddress);
#endif // _UI_CALLBACKS_H_

8
src/uint128.c
View File

@@ -53,11 +53,9 @@ void shiftl128(const uint128_t *const number, uint32_t value, uint128_t *const t
} else if (value < 64) {
UPPER_P(target) = (UPPER_P(number) << value) + (LOWER_P(number) >> (64 - value));
LOWER_P(target) = (LOWER_P(number) << value);
} else if ((128 > value) && (value > 64)) {
} else {
UPPER_P(target) = LOWER_P(number) << (value - 64);
LOWER_P(target) = 0;
} else {
clear128(target);
}
}
@@ -74,11 +72,9 @@ void shiftr128(const uint128_t *const number, uint32_t value, uint128_t *const t
UPPER(result) = UPPER_P(number) >> value;
LOWER(result) = (UPPER_P(number) << (64 - value)) + (LOWER_P(number) >> value);
copy128(target, &result);
} else if ((128 > value) && (value > 64)) {
} else {
LOWER_P(target) = UPPER_P(number) >> (value - 64);
UPPER_P(target) = 0;
} else {
clear128(target);
}
}

8
src/uint256.c
View File

@@ -59,11 +59,9 @@ void shiftl256(const uint256_t *const number, uint32_t value, uint256_t *const t
add128(&tmp1, &tmp2, &UPPER(result));
shiftl128(&LOWER_P(number), value, &LOWER(result));
copy256(target, &result);
} else if ((256 > value) && (value > 128)) {
} else {
shiftl128(&LOWER_P(number), (value - 128), &UPPER_P(target));
clear128(&LOWER_P(target));
} else {
clear256(target);
}
}
@@ -84,11 +82,9 @@ void shiftr256(const uint256_t *const number, uint32_t value, uint256_t *const t
shiftl128(&UPPER_P(number), (128 - value), &tmp2);
add128(&tmp1, &tmp2, &LOWER(result));
copy256(target, &result);
} else if ((256 > value) && (value > 128)) {
} else {
shiftr128(&UPPER_P(number), (value - 128), &LOWER_P(target));
clear128(&UPPER_P(target));
} else {
clear256(target);
}
}

17
src/uint_common.c
View File

@@ -58,3 +58,20 @@ void reverseString(char *const str, uint32_t length) {
str[j] = c;
}
}
int bytes_to_string(char *out, size_t outl, const void *value, size_t len) {
if (outl <= 2) {
// Need at least '0x' and 1 digit
return -1;
}
if (strlcpy(out, "0x", outl) != 2) {
goto err;
}
if (format_hex(value, len, out + 2, outl - 2) < 0) {
goto err;
}
return 0;
err:
*out = '\0';
return -1;
}

5
src/uint_common.h
View File

@@ -21,6 +21,9 @@
#define _UINT_COMMON_H_
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include "format.h"
#define UPPER_P(x) x->elements[0]
#define LOWER_P(x) x->elements[1]
@@ -32,4 +35,6 @@ void read_u64_be(const uint8_t *const in, uint64_t *const out);
uint64_t readUint64BE(const uint8_t *const buffer);
void reverseString(char *const str, uint32_t length);
int bytes_to_string(char *out, size_t outl, const void *value, size_t len);
#endif //_UINT_COMMON_H_

6
src_bagl/ui_flow_getEth2PublicKey.c
View File

@@ -2,9 +2,13 @@
#include "shared_context.h"
#include "ui_callbacks.h"
#include "uint_common.h"
void prepare_eth2_public_key() {
snprintf(strings.tmp.tmp, 100, "0x%.*H", 48, tmpCtx.publicKeyContext.publicKey.W);
bytes_to_string(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
tmpCtx.publicKeyContext.publicKey.W,
48);
}
// clang-format off

4
src_bagl/ui_flow_signMessage712.c
View File

@@ -57,7 +57,7 @@ UX_STEP_INIT(
UX_STEP_CB(
ux_712_step_approve,
pb,
ui_712_approve(NULL),
ui_712_approve(),
{
&C_icon_validate_14,
"Approve",
@@ -65,7 +65,7 @@ UX_STEP_CB(
UX_STEP_CB(
ux_712_step_reject,
pb,
ui_712_reject(NULL),
ui_712_reject(),
{
&C_icon_crossmark,
"Reject",

23
src_bagl/ui_flow_signMessage712_v0.c
View File

@@ -1,21 +1,20 @@
#include "shared_context.h"
#include "ui_callbacks.h"
#include "common_712.h"
#include "uint_common.h"
void prepare_domain_hash_v0() {
snprintf(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
"0x%.*H",
KECCAK256_HASH_BYTESIZE,
tmpCtx.messageSigningContext712.domainHash);
bytes_to_string(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
tmpCtx.messageSigningContext712.domainHash,
KECCAK256_HASH_BYTESIZE);
}
void prepare_message_hash_v0() {
snprintf(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
"0x%.*H",
KECCAK256_HASH_BYTESIZE,
tmpCtx.messageSigningContext712.messageHash);
bytes_to_string(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
tmpCtx.messageSigningContext712.messageHash,
KECCAK256_HASH_BYTESIZE);
}
// clang-format off
@@ -46,7 +45,7 @@ UX_STEP_NOCB_INIT(
UX_STEP_CB(
ux_sign_712_v0_flow_4_step,
pbb,
ui_712_approve_cb(NULL),
ui_712_approve_cb(),
{
&C_icon_validate_14,
"Sign",
@@ -55,7 +54,7 @@ UX_STEP_CB(
UX_STEP_CB(
ux_sign_712_v0_flow_5_step,
pbb,
ui_712_reject_cb(NULL),
ui_712_reject_cb(),
{
&C_icon_crossmark,
"Cancel",

11
src_features/performPrivacyOperation/cmd_performPrivacyOperation.c
View File

@@ -3,6 +3,7 @@
#include "feature_performPrivacyOperation.h"
#include "common_ui.h"
#include "uint_common.h"
#define P2_PUBLIC_ENCRYPTION_KEY 0x00
#define P2_SHARED_SECRET 0x01
@@ -106,11 +107,11 @@ void handlePerformPrivacyOperation(uint8_t p1,
for (uint8_t i = 0; i < 32; i++) {
privateKeyData[i] = tmpCtx.publicKeyContext.publicKey.W[32 - i];
}
snprintf(strings.common.fullAmount,
sizeof(strings.common.fullAmount) - 1,
"%.*H",
32,
privateKeyData);
format_hex(privateKeyData,
32,
strings.common.fullAmount,
sizeof(strings.common.fullAmount) - 1);
if (p2 == P2_PUBLIC_ENCRYPTION_KEY) {
ui_display_privacy_public_key();
} else {

5
src_features/performPrivacyOperation/feature_performPrivacyOperation.h
View File

@@ -1,3 +1,8 @@
#ifndef _PERFORM_PRIVACY_OPERATION_H_
#define _PERFORM_PRIVACY_OPERATION_H_
#include "shared_context.h"
uint32_t set_result_perform_privacy_operation(void);
#endif // _PERFORM_PRIVACY_OPERATION_H_

4
src_features/provideNFTInformation/cmd_provideNFTInfo.c
View File

@@ -137,7 +137,7 @@ void handleProvideNFTInformation(uint8_t p1,
offset += CHAIN_ID_SIZE;
uint8_t keyId = workBuffer[offset];
uint8_t *rawKey;
const uint8_t *rawKey;
uint8_t rawKeyLen;
PRINTF("KeyID: %d\n", keyId);
@@ -146,7 +146,7 @@ void handleProvideNFTInformation(uint8_t p1,
case STAGING_NFT_METADATA_KEY:
#endif
case PROD_NFT_METADATA_KEY:
rawKey = (uint8_t *) LEDGER_NFT_METADATA_PUBLIC_KEY;
rawKey = LEDGER_NFT_METADATA_PUBLIC_KEY;
rawKeyLen = sizeof(LEDGER_NFT_METADATA_PUBLIC_KEY);
break;
default:

12
src_features/signMessageEIP712/ui_logic.c
View File

@@ -17,6 +17,7 @@
#include "commands_712.h"
#include "common_ui.h"
#include "domain_name.h"
#include "uint_common.h"
static t_ui_context *ui_ctx = NULL;
@@ -165,11 +166,10 @@ void ui_712_message_hash(void) {
const char *const title = "Message hash";
ui_712_set_title(title, strlen(title));
snprintf(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
"0x%.*H",
KECCAK256_HASH_BYTESIZE,
tmpCtx.messageSigningContext712.messageHash);
bytes_to_string(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
tmpCtx.messageSigningContext712.messageHash,
KECCAK256_HASH_BYTESIZE);
ui_712_redraw_generic_step();
}
@@ -286,7 +286,7 @@ static bool ui_712_format_bool(const uint8_t *const data, uint8_t length) {
*/
static void ui_712_format_bytes(const uint8_t *const data, uint8_t length) {
if (ui_712_field_shown()) {
snprintf(strings.tmp.tmp, sizeof(strings.tmp.tmp), "0x%.*H", length, data);
bytes_to_string(strings.tmp.tmp, sizeof(strings.tmp.tmp), data, length);
// +2 for the "0x"
// x2 for each byte value is represented by 2 ASCII characters
if ((2 + (length * 2)) > (sizeof(strings.tmp.tmp) - 1)) {

11
src_features/signTx/logic_signTx.c
View File

@@ -9,10 +9,11 @@
#include "ui_callbacks.h"
#include "apdu_constants.h"
#include "crypto_helpers.h"
#include "format.h"
#define ERR_SILENT_MODE_CHECK_FAILED 0x6001
uint32_t splitBinaryParameterPart(char *result, uint8_t *parameter) {
static uint32_t splitBinaryParameterPart(char *result, size_t result_size, uint8_t *parameter) {
uint32_t i;
for (i = 0; i < 8; i++) {
if (parameter[i] != 0x00) {
@@ -25,7 +26,7 @@ uint32_t splitBinaryParameterPart(char *result, uint8_t *parameter) {
result[2] = '\0';
return 2;
} else {
array_hexstr(result, parameter + i, 8 - i);
format_hex(parameter + i, 8 - i, result, result_size);
return ((8 - i) * 2);
}
}
@@ -144,7 +145,10 @@ customStatus_e customProcessor(txContext_t *context) {
}
dataContext.tokenContext.fieldOffset = 0;
if (fieldPos == 0) {
array_hexstr(strings.tmp.tmp, dataContext.tokenContext.data, 4);
format_hex(dataContext.tokenContext.data,
4,
strings.tmp.tmp,
sizeof(strings.tmp.tmp));
ui_confirm_selector();
} else {
uint32_t offset = 0;
@@ -155,6 +159,7 @@ customStatus_e customProcessor(txContext_t *context) {
dataContext.tokenContext.fieldIndex);
for (i = 0; i < 4; i++) {
offset += splitBinaryParameterPart(strings.tmp.tmp + offset,
sizeof(strings.tmp.tmp) - offset,
dataContext.tokenContext.data + 8 * i);
if (i != 3) {
strings.tmp.tmp[offset++] = ':';

6
src_nbgl/ui_get_eth2_public_key.c
View File

@@ -2,6 +2,7 @@
#include "shared_context.h"
#include "ui_callbacks.h"
#include "ui_nbgl.h"
#include "uint_common.h"
static void reviewReject(void) {
io_seproxyhal_touch_address_cancel(NULL);
@@ -21,7 +22,10 @@ static void reviewChoice(bool confirm) {
}
static void buildScreen(void) {
snprintf(strings.tmp.tmp, 100, "0x%.*H", 48, tmpCtx.publicKeyContext.publicKey.W);
bytes_to_string(strings.tmp.tmp,
sizeof(strings.tmp.tmp),
tmpCtx.publicKeyContext.publicKey.W,
48);
nbgl_useCaseAddressConfirmation(strings.tmp.tmp, reviewChoice);
}

3
src_nbgl/ui_sign_712_v0.c
View File

@@ -4,13 +4,14 @@
#include "network.h"
#include "ui_message_signing.h"
#include "ui_signing.h"
#include "uint_common.h"
static nbgl_layoutTagValue_t pairs[2];
static void start_review(void); // forward declaration
static char *format_hash(const uint8_t *hash, char *buffer, size_t buffer_size, size_t offset) {
snprintf(buffer + offset, buffer_size - offset, "0x%.*H", KECCAK256_HASH_BYTESIZE, hash);
bytes_to_string(buffer + offset, buffer_size - offset, hash, KECCAK256_HASH_BYTESIZE);
return buffer + offset;
}

2
src_plugins/erc1155/erc1155_plugin.c
View File

@@ -27,7 +27,7 @@ static void handle_init_contract(void *parameters) {
}
uint8_t i;
for (i = 0; i < SELECTORS_COUNT; i++) {
if (memcmp((uint8_t *) PIC(ERC1155_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
if (memcmp(PIC(ERC1155_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
context->selectorIndex = i;
break;
}

2
src_plugins/erc20/erc20_plugin.c
View File

@@ -64,7 +64,7 @@ const contract_t CONTRACTS[NUM_CONTRACTS] = {
bool check_contract(erc20_parameters_t *context) {
for (size_t i = 0; i < NUM_CONTRACTS; i++) {
contract_t *contract = (contract_t *) PIC(&CONTRACTS[i]);
const contract_t *contract = (const contract_t *) PIC(&CONTRACTS[i]);
if (memcmp(contract->address, context->destinationAddress, ADDRESS_LENGTH) == 0) {
strlcpy(context->contract_name, contract->name, sizeof(context->contract_name));
return true;

2
src_plugins/erc721/erc721_plugin.c
View File

@@ -32,7 +32,7 @@ static void handle_init_contract(void *parameters) {
}
uint8_t i;
for (i = 0; i < SELECTORS_COUNT; i++) {
if (memcmp((uint8_t *) PIC(ERC721_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
if (memcmp(PIC(ERC721_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
context->selectorIndex = i;
break;
}

2
src_plugins/eth2/eth2_plugin.c
View File

@@ -199,7 +199,7 @@ void eth2_plugin_call(int message, void *parameters) {
switch (msg->screenIndex) {
case 0: { // Amount screen
uint8_t decimals = WEI_TO_ETHER;
char *ticker = chainConfig->coinName;
const char *ticker = chainConfig->coinName;
strlcpy(msg->title, "Amount", msg->titleLength);
if (!amountToString(tmpContent.txContent.value.value,
tmpContent.txContent.value.length,