Merge pull request #133 from LedgerHQ/fix_security_and_display_issues

Display Validator address, add bound to withdrawal derivation index, check deposit contract addresss.
This commit is contained in:
Jean P
2021-05-05 00:10:39 +02:00
committed by GitHub
7 changed files with 146 additions and 51 deletions

View File

@@ -56,10 +56,11 @@ void eth_plugin_prepare_query_contract_UI(ethQueryContractUI_t *queryContractUI,
queryContractUI->msgLength = msgLength; queryContractUI->msgLength = msgLength;
} }
int eth_plugin_perform_init(uint8_t *contractAddress, ethPluginInitContract_t *init) { eth_plugin_result_t eth_plugin_perform_init(uint8_t *contractAddress,
ethPluginInitContract_t *init) {
uint8_t i; uint8_t i;
const uint8_t **selectors; const uint8_t **selectors;
dataContext.tokenContext.pluginAvailable = 0; dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_UNAVAILABLE;
// Handle hardcoded plugin list // Handle hardcoded plugin list
PRINTF("Selector %.*H\n", 4, init->selector); PRINTF("Selector %.*H\n", 4, init->selector);
for (i = 0;; i++) { for (i = 0;; i++) {
@@ -74,7 +75,7 @@ int eth_plugin_perform_init(uint8_t *contractAddress, ethPluginInitContract_t *i
if ((INTERNAL_ETH_PLUGINS[i].availableCheck == NULL) || if ((INTERNAL_ETH_PLUGINS[i].availableCheck == NULL) ||
((PluginAvailableCheck) PIC(INTERNAL_ETH_PLUGINS[i].availableCheck))()) { ((PluginAvailableCheck) PIC(INTERNAL_ETH_PLUGINS[i].availableCheck))()) {
strcpy(dataContext.tokenContext.pluginName, INTERNAL_ETH_PLUGINS[i].alias); strcpy(dataContext.tokenContext.pluginName, INTERNAL_ETH_PLUGINS[i].alias);
dataContext.tokenContext.pluginAvailable = 1; dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_OK;
contractAddress = NULL; contractAddress = NULL;
break; break;
} }
@@ -94,23 +95,22 @@ int eth_plugin_perform_init(uint8_t *contractAddress, ethPluginInitContract_t *i
} else { } else {
PRINTF("Trying alias %s\n", dataContext.tokenContext.pluginName); PRINTF("Trying alias %s\n", dataContext.tokenContext.pluginName);
} }
int status = eth_plugin_call(contractAddress, ETH_PLUGIN_INIT_CONTRACT, (void *) init); eth_plugin_result_t status =
if (!status) { eth_plugin_call(contractAddress, ETH_PLUGIN_INIT_CONTRACT, (void *) init);
return 0; if (status <= ETH_PLUGIN_RESULT_UNSUCCESSFUL) {
} return status;
if (status == ETH_PLUGIN_RESULT_OK) { } else if (status == ETH_PLUGIN_RESULT_OK_ALIAS) {
break;
}
if (status == ETH_PLUGIN_RESULT_OK_ALIAS) {
contractAddress = NULL; contractAddress = NULL;
} else {
break;
} }
} }
PRINTF("eth_plugin_init ok %s\n", dataContext.tokenContext.pluginName); PRINTF("eth_plugin_init ok %s\n", dataContext.tokenContext.pluginName);
dataContext.tokenContext.pluginAvailable = 1; dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_OK;
return 1; return ETH_PLUGIN_RESULT_OK;
} }
int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) { eth_plugin_result_t eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
ethPluginSharedRW_t pluginRW; ethPluginSharedRW_t pluginRW;
ethPluginSharedRO_t pluginRO; ethPluginSharedRO_t pluginRO;
char tmp[PLUGIN_ID_LENGTH]; char tmp[PLUGIN_ID_LENGTH];
@@ -122,9 +122,9 @@ int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
pluginRO.txContent = &tmpContent.txContent; pluginRO.txContent = &tmpContent.txContent;
if (contractAddress == NULL) { if (contractAddress == NULL) {
if (!dataContext.tokenContext.pluginAvailable) { if (dataContext.tokenContext.pluginStatus <= ETH_PLUGIN_RESULT_UNSUCCESSFUL) {
PRINTF("Cached plugin call but no plugin available\n"); PRINTF("Cached plugin call but no plugin available\n");
return 0; return dataContext.tokenContext.pluginStatus;
} }
alias = dataContext.tokenContext.pluginName; alias = dataContext.tokenContext.pluginName;
} else { } else {
@@ -176,7 +176,7 @@ int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
break; break;
default: default:
PRINTF("Unknown plugin method %d\n", method); PRINTF("Unknown plugin method %d\n", method);
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
// Perform the call // Perform the call
@@ -222,8 +222,10 @@ int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
break; break;
case ETH_PLUGIN_RESULT_OK_ALIAS: case ETH_PLUGIN_RESULT_OK_ALIAS:
break; break;
case ETH_PLUGIN_RESULT_ERROR:
return ETH_PLUGIN_RESULT_ERROR;
default: default:
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
break; break;
case ETH_PLUGIN_PROVIDE_PARAMETER: case ETH_PLUGIN_PROVIDE_PARAMETER:
@@ -231,8 +233,10 @@ int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
case ETH_PLUGIN_RESULT_OK: case ETH_PLUGIN_RESULT_OK:
case ETH_PLUGIN_RESULT_FALLBACK: case ETH_PLUGIN_RESULT_FALLBACK:
break; break;
case ETH_PLUGIN_RESULT_ERROR:
return ETH_PLUGIN_RESULT_ERROR;
default: default:
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
break; break;
case ETH_PLUGIN_FINALIZE: case ETH_PLUGIN_FINALIZE:
@@ -240,8 +244,10 @@ int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
case ETH_PLUGIN_RESULT_OK: case ETH_PLUGIN_RESULT_OK:
case ETH_PLUGIN_RESULT_FALLBACK: case ETH_PLUGIN_RESULT_FALLBACK:
break; break;
case ETH_PLUGIN_RESULT_ERROR:
return ETH_PLUGIN_RESULT_ERROR;
default: default:
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
break; break;
case ETH_PLUGIN_PROVIDE_TOKEN: case ETH_PLUGIN_PROVIDE_TOKEN:
@@ -249,23 +255,25 @@ int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter) {
case ETH_PLUGIN_RESULT_OK: case ETH_PLUGIN_RESULT_OK:
case ETH_PLUGIN_RESULT_FALLBACK: case ETH_PLUGIN_RESULT_FALLBACK:
break; break;
case ETH_PLUGIN_RESULT_ERROR:
return ETH_PLUGIN_RESULT_ERROR;
default: default:
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
break; break;
case ETH_PLUGIN_QUERY_CONTRACT_ID: case ETH_PLUGIN_QUERY_CONTRACT_ID:
if (((ethQueryContractID_t *) parameter)->result != ETH_PLUGIN_RESULT_OK) { if (((ethQueryContractID_t *) parameter)->result <= ETH_PLUGIN_RESULT_UNSUCCESSFUL) {
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
break; break;
case ETH_PLUGIN_QUERY_CONTRACT_UI: case ETH_PLUGIN_QUERY_CONTRACT_UI:
if (((ethQueryContractUI_t *) parameter)->result != ETH_PLUGIN_RESULT_OK) { if (((ethQueryContractUI_t *) parameter)->result <= ETH_PLUGIN_RESULT_OK) {
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
break; break;
default: default:
return 0; return ETH_PLUGIN_RESULT_UNAVAILABLE;
} }
return 1; return ETH_PLUGIN_RESULT_OK;
} }

View File

@@ -22,9 +22,10 @@ void eth_plugin_prepare_query_contract_UI(ethQueryContractUI_t *queryContractUI,
char *msg, char *msg,
uint32_t msgLength); uint32_t msgLength);
int eth_plugin_perform_init(uint8_t *contractAddress, ethPluginInitContract_t *init); eth_plugin_result_t eth_plugin_perform_init(uint8_t *contractAddress,
ethPluginInitContract_t *init);
// NULL for cached address, or base contract address // NULL for cached address, or base contract address
int eth_plugin_call(uint8_t *contractAddress, int method, void *parameter); eth_plugin_result_t eth_plugin_call(uint8_t *contractAddress, int method, void *parameter);
int compound_plugin_call(uint8_t *contractAddress, int method, void *parameter); int compound_plugin_call(uint8_t *contractAddress, int method, void *parameter);
void plugin_ui_start(void); void plugin_ui_start(void);

View File

@@ -21,11 +21,16 @@ typedef enum {
} eth_plugin_msg_t; } eth_plugin_msg_t;
typedef enum { typedef enum {
// Unsuccesful return values
ETH_PLUGIN_RESULT_ERROR = 0x00, ETH_PLUGIN_RESULT_ERROR = 0x00,
ETH_PLUGIN_RESULT_OK = 0x01, ETH_PLUGIN_RESULT_UNAVAILABLE = 0x01,
ETH_PLUGIN_RESULT_OK_ALIAS = 0x02, ETH_PLUGIN_RESULT_UNSUCCESSFUL = 0x02, // Used for comparison
ETH_PLUGIN_RESULT_FALLBACK = 0x03
// Successful return values
ETH_PLUGIN_RESULT_SUCCESSFUL = 0x03, // Used for comparison
ETH_PLUGIN_RESULT_OK = 0x04,
ETH_PLUGIN_RESULT_OK_ALIAS = 0x05,
ETH_PLUGIN_RESULT_FALLBACK = 0x06
} eth_plugin_result_t; } eth_plugin_result_t;

View File

@@ -48,7 +48,7 @@ typedef enum starkQuantumType_e {
typedef struct tokenContext_t { typedef struct tokenContext_t {
char pluginName[PLUGIN_ID_LENGTH]; char pluginName[PLUGIN_ID_LENGTH];
uint8_t pluginAvailable; uint8_t pluginStatus;
uint8_t data[32]; uint8_t data[32];
uint8_t fieldIndex; uint8_t fieldIndex;

View File

@@ -39,7 +39,7 @@ void handleSign(uint8_t p1,
dataLength -= 4; dataLength -= 4;
} }
dataPresent = false; dataPresent = false;
dataContext.tokenContext.pluginAvailable = 0; dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_UNAVAILABLE;
// EIP 2718: TransactionType might be present before the TransactionPayload. // EIP 2718: TransactionType might be present before the TransactionPayload.
uint8_t txType = *workBuffer; uint8_t txType = *workBuffer;

View File

@@ -48,18 +48,21 @@ customStatus_e customProcessor(txContext_t *context) {
PRINTF("Missing function selector\n"); PRINTF("Missing function selector\n");
return CUSTOM_FAULT; return CUSTOM_FAULT;
} }
dataContext.tokenContext.pluginAvailable = 0; dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_UNAVAILABLE;
// If contract debugging mode is activated, do not go through the plugin activation // If contract debugging mode is activated, do not go through the plugin activation
// as they wouldn't be displayed if the plugin consumes all data but fallbacks // as they wouldn't be displayed if the plugin consumes all data but fallbacks
if (!N_storage.contractDetails) { if (!N_storage.contractDetails) {
eth_plugin_prepare_init(&pluginInit, eth_plugin_prepare_init(&pluginInit,
context->workBuffer, context->workBuffer,
context->currentFieldLength); context->currentFieldLength);
dataContext.tokenContext.pluginAvailable = dataContext.tokenContext.pluginStatus =
eth_plugin_perform_init(tmpContent.txContent.destination, &pluginInit); eth_plugin_perform_init(tmpContent.txContent.destination, &pluginInit);
} }
PRINTF("pluginAvailable %d\n", dataContext.tokenContext.pluginAvailable); PRINTF("pluginstatus %d\n", dataContext.tokenContext.pluginStatus);
if (dataContext.tokenContext.pluginAvailable) { eth_plugin_result_t status = dataContext.tokenContext.pluginStatus;
if (status == ETH_PLUGIN_RESULT_ERROR) {
return CUSTOM_FAULT;
} else if (status >= ETH_PLUGIN_RESULT_SUCCESSFUL) {
dataContext.tokenContext.fieldIndex = 0; dataContext.tokenContext.fieldIndex = 0;
dataContext.tokenContext.fieldOffset = 0; dataContext.tokenContext.fieldOffset = 0;
copyTxData(context, NULL, 4); copyTxData(context, NULL, 4);
@@ -83,7 +86,8 @@ customStatus_e customProcessor(txContext_t *context) {
dataContext.tokenContext.fieldOffset = 0; dataContext.tokenContext.fieldOffset = 0;
blockSize = 4; blockSize = 4;
} else { } else {
if (!N_storage.contractDetails && !dataContext.tokenContext.pluginAvailable) { if (!N_storage.contractDetails &&
dataContext.tokenContext.pluginStatus <= ETH_PLUGIN_RESULT_UNSUCCESSFUL) {
return CUSTOM_NOT_HANDLED; return CUSTOM_NOT_HANDLED;
} }
blockSize = 32 - (dataContext.tokenContext.fieldOffset % 32); blockSize = 32 - (dataContext.tokenContext.fieldOffset % 32);
@@ -112,7 +116,7 @@ customStatus_e customProcessor(txContext_t *context) {
if (copySize == blockSize) { if (copySize == blockSize) {
// Can process or display // Can process or display
if (dataContext.tokenContext.pluginAvailable) { if (dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) {
ethPluginProvideParameter_t pluginProvideParameter; ethPluginProvideParameter_t pluginProvideParameter;
eth_plugin_prepare_provide_parameter(&pluginProvideParameter, eth_plugin_prepare_provide_parameter(&pluginProvideParameter,
dataContext.tokenContext.data, dataContext.tokenContext.data,
@@ -275,7 +279,7 @@ void finalizeParsing(bool direct) {
32); 32);
// Finalize the plugin handling // Finalize the plugin handling
if (dataContext.tokenContext.pluginAvailable) { if (dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) {
genericUI = false; genericUI = false;
eth_plugin_prepare_finalize(&pluginFinalize); eth_plugin_prepare_finalize(&pluginFinalize);
if (!eth_plugin_call(NULL, ETH_PLUGIN_FINALIZE, (void *) &pluginFinalize)) { if (!eth_plugin_call(NULL, ETH_PLUGIN_FINALIZE, (void *) &pluginFinalize)) {

View File

@@ -20,17 +20,54 @@ void getEth2PublicKey(uint32_t *bip32Path, uint8_t bip32PathLength, uint8_t *out
#define ETH2_WITHDRAWAL_CREDENTIALS_LENGTH 0x20 #define ETH2_WITHDRAWAL_CREDENTIALS_LENGTH 0x20
#define ETH2_SIGNATURE_LENGTH 0x60 #define ETH2_SIGNATURE_LENGTH 0x60
static const uint8_t deposit_contract_address[] = {0x00, 0x00, 0x00, 0x00, 0x21, 0x9a, 0xb5,
0x40, 0x35, 0x6c, 0xbb, 0x83, 0x9c, 0xbe,
0x05, 0x30, 0x3d, 0x77, 0x05, 0xfa};
// Highest index for withdrawal derivation path.
#define INDEX_MAX 65536 // 2 ^ 16 : arbitrary value to protect from path attacks.
typedef struct eth2_deposit_parameters_t { typedef struct eth2_deposit_parameters_t {
uint8_t valid; uint8_t valid;
char deposit_address[ETH2_DEPOSIT_PUBKEY_LENGTH];
} eth2_deposit_parameters_t; } eth2_deposit_parameters_t;
static void to_lowercase(char *str, unsigned char size) {
for (unsigned char i = 0; i < size && str[i] != 0; i++) {
if (str[i] >= 'A' && str[i] <= 'Z') {
str[i] += 'a' - 'A';
}
}
}
// Fills the `out` buffer with the lowercase string representation of the pubkey passed in as binary
// format by `in`. Does not check the size, so expects `out` to be big enough to hold the string
// representation. Returns the length of string (counting the null terminating character).
static int getEthDisplayableAddress(char *out, uint8_t *in, cx_sha3_t *sha3) {
out[0] = '0';
out[1] = 'x';
getEthAddressStringFromBinary(in, (uint8_t *) out + 2, sha3, chainConfig);
uint8_t destinationLen = strlen(out) + 1; // Adding one to account for \0.
return destinationLen;
}
void eth2_plugin_call(int message, void *parameters) { void eth2_plugin_call(int message, void *parameters) {
switch (message) { switch (message) {
case ETH_PLUGIN_INIT_CONTRACT: { case ETH_PLUGIN_INIT_CONTRACT: {
ethPluginInitContract_t *msg = (ethPluginInitContract_t *) parameters; ethPluginInitContract_t *msg = (ethPluginInitContract_t *) parameters;
eth2_deposit_parameters_t *context = (eth2_deposit_parameters_t *) msg->pluginContext; eth2_deposit_parameters_t *context = (eth2_deposit_parameters_t *) msg->pluginContext;
context->valid = 1; if (memcmp(deposit_contract_address,
msg->result = ETH_PLUGIN_RESULT_OK; msg->pluginSharedRO->txContent->destination,
sizeof(deposit_contract_address)) != 0) {
PRINTF("eth2plugin: failed to check deposit contract\n");
context->valid = 0;
msg->result = ETH_PLUGIN_RESULT_ERROR;
} else {
context->valid = 1;
msg->result = ETH_PLUGIN_RESULT_OK;
}
} break; } break;
case ETH_PLUGIN_PROVIDE_PARAMETER: { case ETH_PLUGIN_PROVIDE_PARAMETER: {
@@ -83,9 +120,34 @@ void eth2_plugin_call(int message, void *parameters) {
msg->result = ETH_PLUGIN_RESULT_OK; msg->result = ETH_PLUGIN_RESULT_OK;
} break; } break;
case 4 + (32 * 3): // deposit data root case 4 + (32 * 5): // deposit pubkey 1
case 4 + (32 * 5): // deposit pubkey {
case 4 + (32 * 6): // Copy the first 32 bytes.
memcpy(context->deposit_address,
msg->parameter,
sizeof(context->deposit_address));
msg->result = ETH_PLUGIN_RESULT_OK;
break;
}
case 4 + (32 * 6): // deposit pubkey 2
{
// Copy the last 16 bytes.
memcpy(context->deposit_address + 32,
msg->parameter,
sizeof(context->deposit_address) - 32);
// Use a temporary buffer to store the string representation.
char tmp[ETH2_DEPOSIT_PUBKEY_LENGTH];
getEthDisplayableAddress(tmp,
(uint8_t *) context->deposit_address,
msg->pluginSharedRW->sha3);
// Copy back the string to the global variable.
strcpy(context->deposit_address, tmp);
msg->result = ETH_PLUGIN_RESULT_OK;
break;
}
case 4 + (32 * 3): // deposit data root
case 4 + (32 * 10): // signature case 4 + (32 * 10): // signature
case 4 + (32 * 11): case 4 + (32 * 11):
case 4 + (32 * 12): case 4 + (32 * 12):
@@ -98,6 +160,14 @@ void eth2_plugin_call(int message, void *parameters) {
uint32_t withdrawalKeyPath[4]; uint32_t withdrawalKeyPath[4];
withdrawalKeyPath[0] = WITHDRAWAL_KEY_PATH_1; withdrawalKeyPath[0] = WITHDRAWAL_KEY_PATH_1;
withdrawalKeyPath[1] = WITHDRAWAL_KEY_PATH_2; withdrawalKeyPath[1] = WITHDRAWAL_KEY_PATH_2;
if (eth2WithdrawalIndex > INDEX_MAX) {
PRINTF("eth2 plugin: withdrawal index is too big\n");
PRINTF("Got %u which is higher than INDEX_MAX (%u)\n",
eth2WithdrawalIndex,
INDEX_MAX);
msg->result = ETH_PLUGIN_RESULT_ERROR;
context->valid = 0;
}
withdrawalKeyPath[2] = eth2WithdrawalIndex; withdrawalKeyPath[2] = eth2WithdrawalIndex;
withdrawalKeyPath[3] = WITHDRAWAL_KEY_PATH_4; withdrawalKeyPath[3] = WITHDRAWAL_KEY_PATH_4;
getEth2PublicKey(withdrawalKeyPath, 4, tmp); getEth2PublicKey(withdrawalKeyPath, 4, tmp);
@@ -108,9 +178,11 @@ void eth2_plugin_call(int message, void *parameters) {
PRINTF("eth2 plugin invalid withdrawal credentials\n"); PRINTF("eth2 plugin invalid withdrawal credentials\n");
PRINTF("Got %.*H\n", 32, msg->parameter); PRINTF("Got %.*H\n", 32, msg->parameter);
PRINTF("Expected %.*H\n", 32, tmp); PRINTF("Expected %.*H\n", 32, tmp);
msg->result = ETH_PLUGIN_RESULT_ERROR;
context->valid = 0; context->valid = 0;
} else {
msg->result = ETH_PLUGIN_RESULT_OK;
} }
msg->result = ETH_PLUGIN_RESULT_OK;
} break; } break;
default: default:
@@ -124,7 +196,7 @@ void eth2_plugin_call(int message, void *parameters) {
eth2_deposit_parameters_t *context = (eth2_deposit_parameters_t *) msg->pluginContext; eth2_deposit_parameters_t *context = (eth2_deposit_parameters_t *) msg->pluginContext;
PRINTF("eth2 plugin finalize\n"); PRINTF("eth2 plugin finalize\n");
if (context->valid) { if (context->valid) {
msg->numScreens = 1; msg->numScreens = 2;
msg->uiType = ETH_UI_TYPE_GENERIC; msg->uiType = ETH_UI_TYPE_GENERIC;
msg->result = ETH_PLUGIN_RESULT_OK; msg->result = ETH_PLUGIN_RESULT_OK;
} else { } else {
@@ -141,9 +213,9 @@ void eth2_plugin_call(int message, void *parameters) {
case ETH_PLUGIN_QUERY_CONTRACT_UI: { case ETH_PLUGIN_QUERY_CONTRACT_UI: {
ethQueryContractUI_t *msg = (ethQueryContractUI_t *) parameters; ethQueryContractUI_t *msg = (ethQueryContractUI_t *) parameters;
// eth2_deposit_parameters_t *context = (eth2_deposit_parameters_t*)msg->pluginContext; eth2_deposit_parameters_t *context = (eth2_deposit_parameters_t *) msg->pluginContext;
switch (msg->screenIndex) { switch (msg->screenIndex) {
case 0: { case 0: { // Amount screen
uint8_t decimals = WEI_TO_ETHER; uint8_t decimals = WEI_TO_ETHER;
uint8_t *ticker = (uint8_t *) PIC(chainConfig->coinName); uint8_t *ticker = (uint8_t *) PIC(chainConfig->coinName);
strcpy(msg->title, "Amount"); strcpy(msg->title, "Amount");
@@ -155,6 +227,11 @@ void eth2_plugin_call(int message, void *parameters) {
100); 100);
msg->result = ETH_PLUGIN_RESULT_OK; msg->result = ETH_PLUGIN_RESULT_OK;
} break; } break;
case 1: { // Deposit pubkey screen
strcpy(msg->title, "Validator");
strcpy(msg->msg, context->deposit_address);
msg->result = ETH_PLUGIN_RESULT_OK;
}
default: default:
break; break;
} }