What the developer wrote
Authored by Qkin-Keystone
fix review bug
What changed, and why it matters
This commit fixes several bugs in the Tron (TRX) signing flow of a cryptocurrency hardware wallet firmware. The changes remove an unused import, eliminate a catch-all error branch that could hide unsupported transaction types, add a missing master-fingerprint lookup, replace a hardcoded seed buffer size with a named constant, and harden string-copy helpers against NULL pointers and buffer overruns. The most user-visible risk is that the old code could crash or leak memory when displaying Tron personal-message data, and the missing master-fingerprint call could have produced incomplete or incorrect signing responses.
Treat as a routine bug-fix/security-hardening patch. Review whether the missing GetMasterFingerPrint call could have produced malformed UR responses in prior releases, and verify that the refactored string helpers are covered by unit/fuzz tests. No immediate incident response is indicated, but users on affected firmware should update when available.
Security signals we found
Buffer handling hardening in message display helpers (NULL checks, bounded snprintf, ellipsis accounting)
Removal of non-exhaustive catch-all error arm in Rust match, preventing hidden unsupported transaction variants
Addition of missing GetMasterFingerPrint call in Tron data retrieval path
Replacement of magic-number seed buffer with named constant SEED_LEN
Removal of unused TronSignature import
Evidence from the diff
The patch touches three files. In rust/rust_c/src/common/ur.rs an unused TronSignature import is removed. In rust/rust_c/src/tron/mod.rs the _ => fallback in tron_check_sign_request is removed, so non-exhaustive matches will now fail at compile time rather than silently returning an unsupported-transaction error at runtime. In src/ui/gui_chain/multi/web3/gui_trx.c: (1) GetMasterFingerPrint(mfp) is added in GuiGetTrxData before the signing-result path is used; (2) the local seed buffer is changed from a literal 64 to SEED_LEN; (3) the three message-display helpers are refactored into a single CopyTrxMessageWithEllipsis helper that NULL-checks both destination and source, avoids strcat/strcpy_s mixing, and correctly reserves space for the ellipsis and a trailing warning string. The old code dereferenced message->from/utf8_message/raw_message without NULL checks and used unsafe snprintf/strcat patterns that could truncate without NUL-termination or write past the buffer.
Changed components
Keystone 3 firmware Tron (TRX) signing UIrust/rust_c/src/common/ur.rsrust/rust_c/src/tron/mod.rssrc/ui/gui_chain/multi/web3/gui_trx.cInspect captured patch +38 / −27
diff --git a/rust/rust_c/src/common/ur.rs b/rust/rust_c/src/common/ur.rs
index 1f3d451..4b23ef5 100644
--- a/rust/rust_c/src/common/ur.rs
+++ b/rust/rust_c/src/common/ur.rs
@@ -71,8 +71,6 @@ use ur_registry::sui::sui_sign_request::SuiSignRequest;
use ur_registry::ton::ton_sign_request::TonSignRequest;
#[cfg(feature = "tron")]
use ur_registry::tron::tron_sign_request::TronSignRequest;
-#[cfg(feature = "tron")]
-use ur_registry::tron::tron_signature::TronSignature;
#[cfg(feature = "zcash")]
use ur_registry::zcash::zcash_pczt::ZcashPczt;
diff --git a/rust/rust_c/src/tron/mod.rs b/rust/rust_c/src/tron/mod.rs
index ae6abb0..9011104 100644
--- a/rust/rust_c/src/tron/mod.rs
+++ b/rust/rust_c/src/tron/mod.rs
@@ -66,10 +66,6 @@ pub unsafe extern "C" fn tron_check_sign_request(
}
}
TransactionType::PersonalMessage => TransactionCheckResult::new().c_ptr(),
- _ => TransactionCheckResult::from(RustCError::UnsupportedTransaction(
- "Unsupported Transaction Type".to_string(),
- ))
- .c_ptr(),
}
}
diff --git a/src/ui/gui_chain/multi/web3/gui_trx.c b/src/ui/gui_chain/multi/web3/gui_trx.c
index d0734d5..487136b 100644
--- a/src/ui/gui_chain/multi/web3/gui_trx.c
+++ b/src/ui/gui_chain/multi/web3/gui_trx.c
@@ -46,6 +46,7 @@ void *GuiGetTrxData(void)
void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
QRCodeType urType = g_isMulti ? g_urMultiResult->ur_type : g_urResult->ur_type;
char *trxXpub = GetCurrentAccountPublicKey(XPUB_TYPE_TRX);
+ GetMasterFingerPrint(mfp);
do {
PtrT_TransactionParseResult_DisplayTron parseResult = NULL;
if( urType == KeystoneSignRequest) {
@@ -139,7 +140,7 @@ static UREncodeResult *GuiGetTrxSignUrDataDynamic(bool unLimit)
QRCodeType urType = g_isMulti ? g_urMultiResult->ur_type : g_urResult->ur_type;
uint8_t mfp[4];
GetMasterFingerPrint(mfp);
- uint8_t seed[64];
+ uint8_t seed[SEED_LEN];
uint32_t fragmentLen = unLimit ? FRAGMENT_UNLIMITED_LENGTH : FRAGMENT_MAX_LENGTH_DEFAULT;
do {
@@ -213,40 +214,56 @@ void GetTrxPersonalMessageType(void *indata, void *param, uint32_t maxLen)
}
}
-void GetTrxMessageFrom(void *indata, void *param, uint32_t maxLen)
+static void CopyTrxMessageWithEllipsis(char *dest, uint32_t maxLen, const char *src)
{
- DisplayTRONPersonalMessage *message = (DisplayTRONPersonalMessage *)param;
- if (message->from == NULL) {
- strcpy_s((char *)indata, maxLen, "");
+ if (dest == NULL || maxLen == 0) {
return;
}
- if (strlen(message->from) >= maxLen) {
- snprintf((char *)indata, maxLen - 3, "%s", message->from);
- strcat((char *)indata, "...");
- } else {
- strcpy_s((char *)indata, maxLen, message->from);
+ if (src == NULL) {
+ dest[0] = '\0';
+ return;
}
+ size_t src_len = strlen(src);
+ if (src_len < maxLen) {
+ snprintf(dest, maxLen, "%s", src);
+ return;
+ }
+ if (maxLen <= 4) {
+ snprintf(dest, maxLen, "%.*s", (int)(maxLen - 1), "...");
+ return;
+ }
+ snprintf(dest, maxLen, "%.*s...", (int)(maxLen - 4), src);
+}
+
+void GetTrxMessageFrom(void *indata, void *param, uint32_t maxLen)
+{
+ DisplayTRONPersonalMessage *message = (DisplayTRONPersonalMessage *)param;
+ CopyTrxMessageWithEllipsis((char *)indata, maxLen, message ? message->from : NULL);
}
+
void GetTrxMessageUtf8(void *indata, void *param, uint32_t maxLen)
{
DisplayTRONPersonalMessage *message = (DisplayTRONPersonalMessage *)param;
- if (strlen(message->utf8_message) >= maxLen) {
- snprintf((char *)indata, maxLen - 3, "%s", message->utf8_message);
- strcat((char *)indata, "...");
- } else {
- snprintf((char *)indata, maxLen, "%s", message->utf8_message);
- }
+ CopyTrxMessageWithEllipsis((char *)indata, maxLen, message ? message->utf8_message : NULL);
}
void GetTrxMessageRaw(void *indata, void *param, uint32_t maxLen)
{
- int len = strlen("\n#F5C131 The data is not parseable. Please#\n#F5C131 refer to the software wallet interface#\n#F5C131 for viewing.#");
+ const char *warning = "\n#F5C131 The data is not parseable. Please#\n#F5C131 refer to the software wallet interface#\n#F5C131 for viewing.#";
+ size_t warningLen = strlen(warning);
DisplayTRONPersonalMessage *message = (DisplayTRONPersonalMessage *)param;
- if (strlen(message->raw_message) >= maxLen - len) {
- snprintf((char *)indata, maxLen - 3 - len, "%s", message->raw_message);
- strcat((char *)indata, "...");
+ size_t rawLen = message && message->raw_message ? strlen(message->raw_message) : 0;
+ if (maxLen == 0) {
+ return;
+ }
+ if (rawLen + warningLen >= (size_t)maxLen) {
+ if (maxLen <= 4) {
+ snprintf((char *)indata, maxLen, "%s", "");
+ return;
+ }
+ snprintf((char *)indata, maxLen, "%.*s...", (int)(maxLen - 4), message && message->raw_message ? message->raw_message : "");
} else {
- snprintf((char *)indata, maxLen, "%s%s", message->raw_message, "\n#F5C131 The data is not parseable. Please#\n#F5C131 refer to the software wallet interface#\n#F5C131 for viewing.#");
+ snprintf((char *)indata, maxLen, "%s%s", message && message->raw_message ? message->raw_message : "", warning);
}
}
Why this scored 38/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.