What the developer wrote
Authored by ww3512687
review tron
What changed, and why it matters
This commit makes two clean-up changes to Tron transaction handling in the Keystone 3 firmware. First, it avoids calling the same formatting functions twice by reusing computed values. Second, it improves how the signing code handles the wallet's secret seed: it now initializes the seed buffer to the correct size, checks whether retrieving the seed succeeded before using it, and securely wipes the seed from memory even if signing fails. These are defensive hardening improvements rather than a fix for a known active exploit, but they reduce the risk that a failed seed retrieval or leftover seed data could lead to incorrect signing or information leakage.
Treat as a defensive hardening patch. Review whether other chain signing flows (e.g., BTC, ETH, SOL) use the same pattern of unchecked `GetAccountSeed()` return values, fixed-size seed buffers, and missing post-failure wipes, and apply consistent fixes. Verify that `SEED_LEN` matches the maximum possible seed/entropy length used by any account type to avoid truncation.
Security signals we found
Secret seed buffer size changed from hard-coded 64 to project-defined SEED_LEN
Return value of GetAccountSeed() is now checked before signing
Seed buffer is now explicitly zeroed with memset_s on both success and failure paths
ClearSecretCache() moved outside the do/while block so it always runs
Seed length passed to signing function now uses GetCurrentAccountSeedLen() instead of sizeof(local buffer)
Debug printf removed from GuiGetTrxCheckResult
Evidence from the diff
The Rust parser change refactors WrappedTron::parse() to call format_amount(), format_method(), and string conversions once, cloning the results into both OverviewTx and DetailTx. This is a non-functional refactor that removes duplicated computation. The C signing change in GuiGetTrxSignQrCodeData() is more security-relevant: it changes the seed buffer from a hard-coded 64 bytes to SEED_LEN, checks the return value of GetAccountSeed(), moves ClearSecretCache() and a new memset_s(seed, ...) wipe outside the do { ... } while(0) block so the wipe happens on both success and failure paths, and replaces sizeof(seed) with GetCurrentAccountSeedLen() when calling tron_sign_keystone(). These changes reduce the chance of using stale/incorrect seed length or unzeroed seed material if seed retrieval fails.
Changed components
rust/apps/tron/src/transaction/parser.rssrc/ui/gui_chain/multi/web3/gui_trx.cTron signing flowSeed/secret handling in Tron UIInspect captured patch +28 / −20
diff --git a/rust/apps/tron/src/transaction/parser.rs b/rust/apps/tron/src/transaction/parser.rs
index a0ec03a..84124e1 100644
--- a/rust/apps/tron/src/transaction/parser.rs
+++ b/rust/apps/tron/src/transaction/parser.rs
@@ -34,19 +34,25 @@ pub trait TxParser {
impl TxParser for WrappedTron {
fn parse(&self) -> Result<ParsedTx> {
+ let value = self.format_amount()?;
+ let method = self.format_method()?;
+ let from = self.from.to_string();
+ let to = self.to.to_string();
+ let network = NETWORK.to_string();
+
let overview = OverviewTx {
- value: self.format_amount()?,
- method: self.format_method()?,
- from: self.from.to_string(),
- to: self.to.to_string(),
- network: NETWORK.to_string(),
+ value: value.clone(),
+ method: method.clone(),
+ from: from.clone(),
+ to: to.clone(),
+ network: network.clone(),
};
let detail = DetailTx {
- value: self.format_amount()?,
- method: self.format_method()?,
- from: self.from.to_string(),
- to: self.to.to_string(),
- network: NETWORK.to_string(),
+ value,
+ method,
+ from,
+ to,
+ network,
contract_address: self.contract_address.to_string(),
token: self.token.to_string(),
};
diff --git a/src/ui/gui_chain/multi/web3/gui_trx.c b/src/ui/gui_chain/multi/web3/gui_trx.c
index ef3e076..25b1096 100644
--- a/src/ui/gui_chain/multi/web3/gui_trx.c
+++ b/src/ui/gui_chain/multi/web3/gui_trx.c
@@ -45,7 +45,6 @@ void *GuiGetTrxData(void)
PtrT_TransactionCheckResult GuiGetTrxCheckResult(void)
{
- printf("GuiGetTrxCheckResult\r\n");
uint8_t mfp[4];
void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
QRCodeType urType = g_isMulti ? g_urMultiResult->ur_type : g_urResult->ur_type;
@@ -112,20 +111,23 @@ UREncodeResult *GuiGetTrxSignQrCodeData(void)
{
bool enable = IsPreviousLockScreenEnable();
SetLockScreen(false);
- UREncodeResult *encodeResult;
+ UREncodeResult *encodeResult = NULL;
void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
QRCodeType urType = g_isMulti ? g_urMultiResult->ur_type : g_urResult->ur_type;
+ uint8_t mfp[4];
+ GetMasterFingerPrint(mfp);
+ uint8_t seed[SEED_LEN];
do {
- uint8_t mfp[4];
- GetMasterFingerPrint(mfp);
- uint8_t seed[64];
- GetAccountSeed(GetCurrentAccountIndex(), seed, SecretCacheGetPassword());
- char *xPub = GetCurrentAccountPublicKey(XPUB_TYPE_TRX);
- int len = GetMnemonicType() == MNEMONIC_TYPE_BIP39 ? sizeof(seed) : GetCurrentAccountEntropyLen();
- encodeResult = tron_sign_keystone(data, urType, mfp, sizeof(mfp), xPub, SOFTWARE_VERSION, seed, len);
- ClearSecretCache();
+ int ret = GetAccountSeed(GetCurrentAccountIndex(), seed, SecretCacheGetPassword());
+ if (ret != 0) {
+ break;
+ }
+ encodeResult = tron_sign_keystone(data, urType, mfp, sizeof(mfp), GetCurrentAccountPublicKey(XPUB_TYPE_TRX),
+ SOFTWARE_VERSION, seed, GetCurrentAccountSeedLen());
CHECK_CHAIN_BREAK(encodeResult);
} while (0);
+ memset_s(seed, sizeof(seed), 0, sizeof(seed));
+ ClearSecretCache();
SetLockScreen(enable);
return encodeResult;
}
\ No newline at end of file
Why this scored 42/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.