From 60c6c0d78bed8ab2ec4010c3290d94856064b31b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Thu, 13 Aug 2026 20:05:58 +0300 Subject: [PATCH] feat(module): verify Tron structurally on the signing side too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host moved to `verify_contract` in tinyhumansai/openhuman#5533, but the module — the side that actually holds the key and produces the signature — was still calling `verify_transfer`. That check searches for the recipient and the amount as byte runs anywhere in `raw_data`, so a node can pay someone else and leave the requested address in an unrelated field and still get signed. Both call sites now use `verify_contract`, which parses the protobuf and reads the fields that will execute. The new test builds exactly that transaction and asserts the precondition first: assert!( tx::tron::verify_transfer(&raw_data_hex, REQUESTED, &expected_txid, &transfer).is_ok(), "precondition: the byte-run search is fooled by the decoy" ); so it fails if the gap it guards ever stops existing, rather than passing vacuously. `build_unsigned` then refuses it with "does not pay the requested recipient". Checking it host-side is not enough. A host is precisely what a caller could be lying to, and the module's whole purpose is to be the boundary the key sits behind — which is also why the second call site, `attach_signature`, verifies again rather than trusting the first. `fee_limit_sun` is `None` because the wire spec does not carry it: only the host knows what it pinned in its `createtransaction` request, and it checks that before handing the spec over. Every other field is checked here. Co-authored-by: Medulla --- crates/tinywallet-module/src/service/mod.rs | 18 +++- crates/tinywallet-module/src/service/test.rs | 95 ++++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/crates/tinywallet-module/src/service/mod.rs b/crates/tinywallet-module/src/service/mod.rs index 648d445..a085408 100644 --- a/crates/tinywallet-module/src/service/mod.rs +++ b/crates/tinywallet-module/src/service/mod.rs @@ -148,7 +148,18 @@ fn build_unsigned(request: &SigningRequest) -> Result Result { // Verified again rather than trusted from the first call: the two // requests are independent, and a host could reach this one with - // different bytes than the digest was computed over. - tx::tron::verify_transfer(raw_data_hex, expected_to, expected_txid, transfer) + // different bytes than the digest was computed over. Structurally, + // for the same reason as the sign path above. + tx::tron::verify_contract(raw_data_hex, expected_to, expected_txid, transfer, None) .map_err(|e| Failure::InvalidInput(e.to_string()))?; let (rs, recovery) = single_secp256k1(&request.signatures)?; let signature = diff --git a/crates/tinywallet-module/src/service/test.rs b/crates/tinywallet-module/src/service/test.rs index dc443f6..9922f8b 100644 --- a/crates/tinywallet-module/src/service/test.rs +++ b/crates/tinywallet-module/src/service/test.rs @@ -287,6 +287,101 @@ fn a_tron_transaction_whose_txid_does_not_match_its_bytes_is_refused() { assert!(rendered.contains("InvalidInput"), "{rendered}"); } +#[test] +fn a_tron_transaction_paying_a_decoy_recipient_is_refused_on_the_signing_side() { + // The case that motivated moving this module off `verify_transfer`: the + // requested address IS present in `raw_data`, but as an unrelated trailing + // field, while `to_address` pays someone else. A byte-run search over the + // hex is satisfied by the decoy and would have signed it. + // + // The host checks this too, but the check that matters is the one on the + // side holding the key — a host is exactly what a caller could be lying to. + fn varint(mut v: u64) -> Vec { + let mut out = Vec::new(); + loop { + let mut b = (v & 0x7f) as u8; + v >>= 7; + if v != 0 { + b |= 0x80; + } + out.push(b); + if v == 0 { + return out; + } + } + } + fn tagged(number: u64, wire: u64) -> Vec { + varint((number << 3) | wire) + } + fn bytes_field(number: u64, payload: &[u8]) -> Vec { + let mut out = tagged(number, 2); + out.extend(varint(payload.len() as u64)); + out.extend(payload); + out + } + fn addr(a: &str) -> Vec { + let h = tinywallet::address::tron::to_hex(a).unwrap(); + (0..h.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&h[i..i + 2], 16).unwrap()) + .collect() + } + + const REQUESTED: &str = "TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t"; + const ATTACKER: &str = "TLyqzVGLV1srkB7dToTAEqgDSfPtXRJZYH"; + + // TransferContract paying the attacker, for the requested amount. + let mut payload = bytes_field(2, &addr(ATTACKER)); + payload.extend({ + let mut f = tagged(3, 0); + f.extend(varint(1_000_000)); + f + }); + let mut any = bytes_field(1, b"type.googleapis.com/protocol.TransferContract"); + any.extend(bytes_field(2, &payload)); + let mut contract = { + let mut f = tagged(1, 0); + f.extend(varint(1)); + f + }; + contract.extend(bytes_field(2, &any)); + let mut raw = bytes_field(11, &contract); + // The decoy: the requested recipient, somewhere harmless. + raw.extend(bytes_field(99, &addr(REQUESTED))); + + let raw_data_hex = hex(&raw); + let expected_txid = tx::tron::recompute_txid(&raw_data_hex).unwrap(); + let transfer = tinywallet::wire::TronTransfer::Native { + amount_sun: 1_000_000, + }; + + // The old check would have passed this. + assert!( + tx::tron::verify_transfer(&raw_data_hex, REQUESTED, &expected_txid, &transfer).is_ok(), + "precondition: the byte-run search is fooled by the decoy" + ); + + let error = build_unsigned(&SigningRequest { + transaction: TransactionSpec::Tron { + raw_data_hex, + expected_to: REQUESTED.to_string(), + expected_txid, + transfer, + }, + public_key: PublicKey { + key_hex: compressed_public(&evm_key()), + }, + }) + .unwrap_err(); + + let rendered = format!("{error:?}"); + assert!(rendered.contains("InvalidInput"), "{rendered}"); + assert!( + rendered.contains("does not pay the requested recipient"), + "{rendered}" + ); +} + #[test] fn the_exported_names_are_the_published_ones() { // A host resolves the module by these strings; changing either is a