Skip to content

Commit

Permalink
Merge pull request #327 from keepkey/improve-eip712
Browse files Browse the repository at this point in the history 
eip-712 improvements
  • Loading branch information
@pastaghost
pastaghost authored Dec 3, 2022
2 parents 34d42a9 + 82613db commit 8fd103b
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 54 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.7.2)

project(KeepKeyFirmware

VERSION 7.5.0
VERSION 7.5.1

LANGUAGES C CXX ASM)

Expand Down
2 changes: 1 addition & 1 deletion deps/python-keepkey
Submodule python-keepkey updated 2 files
+103 −10 tests/eip712tests.json
+1 −1 tests/test_verify_typed_data.py
127 changes: 78 additions & 49 deletions lib/firmware/eip712.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
requires all complete json message strings to be enclosed by braces, i.e., { ... }
Cannot have entire json string quoted, i.e., "{ ... }" will not work.
Remove all quote escape chars, e.g., {"types": not {\"types\":
int values must be hex. Negative sign indicates negative value, e.g., -5, -8a67
ints: Strings representing ints must fit into a long size (64-bits).
Note: Do not prefix ints or uints with 0x
All hex and byte strings must be big-endian
Byte strings and address should be prefixed by 0x
Expand Down Expand Up @@ -320,44 +320,64 @@ int confirmValue(const char *value) {
return SUCCESS;
}

int dsConfirm(const char *value) {
static const char *name=NULL, *version=NULL, *chainId=NULL, *verifyingContract=NULL;
static const char *dsname=NULL, *dsversion=NULL, *dschainId=NULL, *dsverifyingContract=NULL;
void marshallDsVals(const char *value) {

if (0 == strncmp(nameForValue, "name", sizeof("name"))) {
name = value;
dsname = value;
}
if (0 == strncmp(nameForValue, "version", sizeof("version"))) {
version = value;
dsversion = value;
}
if (0 == strncmp(nameForValue, "chainId", sizeof("chainId"))) {
chainId = value;
dschainId = value;
}
if (0 == strncmp(nameForValue, "verifyingContract", sizeof("verifyingContract"))) {
verifyingContract = value;
dsverifyingContract = value;
}
return;
}

void dsConfirm(void) {
// First check if we recognize the contract
const TokenType *assetToken;
uint8_t addrHexStr[20] = {0};
char name[41] = {0};
char version[11] = {0};
uint32_t chainInt;
bool noChain = true;
int ctr;
IconType iconNum = NO_ICON;
char title[64] = {0};
char *fillerStr = "";
char chainStr[33] = {0};
char verifyingContract[65] = {0};

if (name != NULL && version != NULL && chainId != NULL && verifyingContract != NULL) {
// First check if we recognize the contract
const TokenType *assetToken;
uint8_t addrHexStr[20];
uint32_t chainInt;
int ctr;
IconType iconNum = NO_ICON;
char title[33] = {0};
char *fillerStr = NULL;
char chainStr[33];
if (dsname != NULL) {
strncpy(name, dsname, 40);
}
if (dsversion != NULL) {
strncpy(version, dsversion, 10);
}

if (dsverifyingContract != NULL) {
for (ctr=2; ctr<42; ctr+=2) {
sscanf((char *)&verifyingContract[ctr], "%2hhx", &addrHexStr[(ctr-2)/2]);
sscanf((char *)&dsverifyingContract[ctr], "%2hhx", &addrHexStr[(ctr-2)/2]);
}
sscanf((char *)chainId, "%ld", &chainInt);
strcat(verifyingContract, "Verifying Contract: ");
strncat(verifyingContract, dsverifyingContract, sizeof(verifyingContract) - sizeof("Verifying Contract: "));
}

if (NULL != dschainId) {
noChain = false;
sscanf((char *)dschainId, "%ld", &chainInt);
// As more chains are supported, add icon choice below
// TBD: not implemented for first release
// if (chainInt == 1) {
// iconNum = ETHEREUM_ICON;
// }

}
if (noChain == false && dsverifyingContract != NULL) {
assetToken = tokenByChainAddress(chainInt, (uint8_t *)addrHexStr);
if (strncmp(assetToken->ticker, " UNKN", 5) == 0) {
fillerStr = "";
Expand All @@ -366,21 +386,23 @@ int dsConfirm(const char *value) {
//fillerStr = "\n\n";
fillerStr = "";
}
strncpy(title, name, 20);
strncat(title, " version ", 32-strlen(title));
strncat(title, version, 32-strlen(title));
if (iconNum == NO_ICON) {
snprintf(chainStr, 32, "chain %s, ", chainId);
}
//snprintf(contractStr, 64, "verifyingContract: %s", verifyingContract);
(void)review_with_icon(ButtonRequestType_ButtonRequest_Other, iconNum,
title, "%sverifyingContract: %s%s", chainStr, verifyingContract, fillerStr);
name = NULL;
version = NULL;
chainId = NULL;
verifyingContract = NULL;
}
return SUCCESS;

strncpy(title, name, 40);
if (NULL != dsversion) {
strncat(title, " Ver: ", 63-strlen(title));
strncat(title, version, 63-strlen(title));
}
if (NULL != dschainId) {
snprintf(chainStr, 32, "chain %s, ", dschainId);
}
//snprintf(contractStr, 64, "verifyingContract: %s", verifyingContract);
(void)review_with_icon(ButtonRequestType_ButtonRequest_Other, iconNum,
title, "%s %s%s", chainStr, verifyingContract, fillerStr);
dsname = NULL;
dsversion = NULL;
dschainId = NULL;
dsverifyingContract = NULL;
}

/*
Expand All @@ -401,7 +423,6 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
const char *typeName = NULL, *typeType = NULL;
uint8_t encBytes[32] = {0}; // holds the encrypted bytes for the message
const char *valStr = NULL;
char byteStrBuf[3] = {0};
struct SHA3_CTX valCtx = {0}; // local hash context
bool hasValue = 0;
bool ds_vals = 0; // domain sep values are confirmed on a single screen
Expand Down Expand Up @@ -457,7 +478,6 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
errRet = JSON_TYPE_WNOVAL;
return errRet;
} else {

if (0 == strncmp("address", typeType, strlen("address")-1)) {
if (']' == typeType[strlen(typeType)-1]) {
// array of addresses
Expand All @@ -466,7 +486,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
while (0 != addrVals) {
// just walk the string values assuming, for fixed sizes, all values are there.
if (ds_vals) {
dsConfirm(json_getValue(addrVals));
marshallDsVals(json_getValue(addrVals));
} else {
confirmValue(json_getValue(addrVals));
}
Expand All @@ -481,7 +501,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
keccak_Final(&valCtx, encBytes);
} else {
if (ds_vals) {
dsConfirm(valStr);
marshallDsVals(valStr);
} else {
confirmValue(valStr);
}
Expand All @@ -500,7 +520,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
while (0 != stringVals) {
// just walk the string values assuming, for fixed sizes, all values are there.
if (ds_vals) {
dsConfirm(json_getValue(stringVals));
marshallDsVals(json_getValue(stringVals));
} else {
confirmValue(json_getValue(stringVals));
}
Expand All @@ -514,7 +534,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
keccak_Final(&valCtx, encBytes);
} else {
if (ds_vals) {
dsConfirm(valStr);
marshallDsVals(valStr);
} else {
confirmValue(valStr);
}
Expand All @@ -531,7 +551,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
return INT_ARRAY_ERROR;
} else {
if (ds_vals) {
dsConfirm(valStr);
marshallDsVals(valStr);
} else {
confirmValue(valStr);
}
Expand All @@ -551,11 +571,17 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
encBytes[ctr] = 0;
}
}
unsigned zeroFillLen = 32 - ((strlen(valStr)-negInt)/2+1);
for (ctr=zeroFillLen; ctr<32; ctr++) {
strncpy(byteStrBuf, &valStr[2*(ctr-(zeroFillLen))], 2);
encBytes[ctr] = (uint8_t)(strtol(byteStrBuf, NULL, 16));
}
// all int strings are assumed to be base 10 and fit into 64 bits
long long intVal = strtoll(valStr, NULL, 10);
// Needs to be big endian, so add to encBytes appropriately
encBytes[24] = (intVal >> 56) & 0xff;
encBytes[25] = (intVal >> 48) & 0xff;
encBytes[26] = (intVal >> 40) & 0xff;
encBytes[27] = (intVal >> 32) & 0xff;
encBytes[28] = (intVal >> 24) & 0xff;
encBytes[29] = (intVal >> 16) & 0xff;
encBytes[30] = (intVal >> 8) & 0xff;
encBytes[31] = (intVal) & 0xff;
}

} else if (0 == strncmp("bytes", typeType, strlen("bytes"))) {
Expand All @@ -564,7 +590,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
} else {
// This could be 'bytes', 'bytes1', ..., 'bytes32'
if (ds_vals) {
dsConfirm(valStr);
marshallDsVals(valStr);
} else {
confirmValue(valStr);
}
Expand All @@ -587,7 +613,7 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
return BOOL_ARRAY_ERROR;
} else {
if (ds_vals) {
dsConfirm(valStr);
marshallDsVals(valStr);
} else {
confirmValue(valStr);
}
Expand Down Expand Up @@ -693,6 +719,10 @@ int parseVals(const json_t *eip712Types, const json_t *jType, const json_t *next
}
tarray = json_getSibling(tarray);
}
if (ds_vals) {
dsConfirm();
}

return SUCCESS;
}

Expand Down Expand Up @@ -768,4 +798,3 @@ int encode(const json_t *jsonTypes, const json_t *jsonVals, const char *typeS, u

return SUCCESS;
}

2 changes: 1 addition & 1 deletion tools/bootloader/usb_flash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ bool usb_flash_firmware(void) {
layout_debuglink_watermark();
#endif

usbInit("app.shapeshift.com");
usbInit("keepkey.com");
bootloader_fsm_init();

while (1) {
Expand Down
2 changes: 1 addition & 1 deletion tools/emulator/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ int main(void) {

led_func(SET_GREEN_LED);

usbInit("app.shapeshift.com");
usbInit("keepkey.com");
led_func(CLR_RED_LED);

reset_idle_time();
Expand Down
2 changes: 1 addition & 1 deletion tools/firmware/keepkey_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ int main(void) {

led_func(SET_GREEN_LED);

usbInit("app.shapeshift.com");
usbInit("keepkey.com");

u2fInit();
led_func(CLR_RED_LED);
Expand Down

0 comments on commit 8fd103b

Please sign in to comment.