Skip to content

fix: F-2026-18177 | [Dual Defense] User-Selected Rescue PRC20 Can Reinterpret a Stuck Inbound as Another Vault Asset - #343

Merged
0xNilesh merged 1 commit into
audit-fixesfrom
F-2026-18177
Aug 26, 2026
Merged

fix: F-2026-18177 | [Dual Defense] User-Selected Rescue PRC20 Can Reinterpret a Stuck Inbound as Another Vault Asset#343
0xNilesh merged 1 commit into
audit-fixesfrom
F-2026-18177

Conversation

@0xNilesh

Copy link
Copy Markdown
Member

F-2026-18177 — User-Selected Rescue PRC20 Can Reinterpret a Stuck Inbound as Another Vault Asset

Severity: Low (Impact 1 / Likelihood 1) · Repo: push-chain-node · Module: x/uexecutor

The bug

In AttachRescueOutboundFromReceipt (x/uexecutor/keeper/create_outbound.go), the rescue outbound
took its asset identity from the caller's event and its amount from the original inbound:

tokenCfg, err := k.uregistryKeeper.GetTokenConfigByPRC20(
    ctx, originalUtx.InboundTx.SourceChain, event.PRC20)   // caller-selected
...
Amount:            originalUtx.InboundTx.Amount,           // the ORIGINAL stuck deposit
ExternalAssetAddr: tokenCfg.Address,
Prc20AssetAddr:    event.PRC20,

The only validation was that event.PRC20 is some registered PRC20 on that chain — never that it is
this deposit's asset. So a rescue could pair asset B's identity with asset A's raw amount.

Why it matters: raw integers across differing decimals

Amounts are raw base units, so a decimals mismatch amplifies the substitution. A stuck 1 pETH is
1e18 raw. Pointed at 6-decimal USDC, that same 1e18 becomes a claim on 1e18 / 1e6 = 10^12
whole USDC
— bounded only by vault liquidity, and paid for by other depositors.

The realistic actor is not a third party: it is the legitimate owner of a genuinely stuck deposit who
names a more valuable token. The eligibility gates (CEA deposit FAILED; non-CEA INBOUND_REVERT in
REVERTED) check that a rescue is warranted, never which asset — they are correct and unchanged.

The fix

1. Derive, do not accept. The asset now resolves from originalUtx.InboundTx.AssetAddr via
GetTokenConfig(ctx, chain, address), and the PRC20 comes from that config's
NativeRepresentation.ContractAddress. This is the same rule the PC20 branch on develop /
testnet/donut already implements
— its comment states detection is "on the original inbound's
is_pc20, not the rescue event"
. The reasoning already existed in the file; it simply had not been
carried into the sibling branch. This change brings the two into line. (audit-fixes predates the
PC20 work and has only the non-PC20 branch, so the diff here is a single site.)

2. Defence in depth: reject disagreement, don't override. The event still carries a PRC20. If it
disagrees with the derived one, the rescue is rejected with an actionable error rather than
silently overridden. Overriding would hide a mismatched client; rejecting surfaces it. This is the
assertion that would have caught the finding.

Comparison uses utils.LenientCanonicalizeEVMAddress (PRC20s are Push-side EVM addresses): it mirrors
uregistry's own PRC20 identity function (canonicalPRC20, which falls back to lowercase-trim when an
address doesn't parse), so exactly the pairs the registry considers equal are accepted, and a
legitimate rescue can never be rejected merely because a registry entry doesn't parse. A missing or
empty NativeRepresentation is rejected explicitly before the comparison, so "missing" can never
read as "match".

3. An original asset with no registered token config is now an error — not a substitution
opportunity. Falls out of (1); the error names the asset and chain.

Declined

Recs 4/5 (source-chain deposit accounting keyed by universalTxId; Vault/Gateway must match the
recorded token+amount) are declined: a substantial cross-repo change, and once the asset is derived
rather than supplied there is nothing left for that accounting to protect against.

Out of scope and unchanged: rescue eligibility gates, the duplicate-rescue guard, the gateway contracts.

Tests

test/integration/uexecutor/rescue_funds_test.go:

  • Headline negative test — a rescue naming a different registered PRC20 than the original
    inbound's asset is rejected, using the 18 → 6 decimal pair where the amplification lives (a
    pETH-style 18-decimal original with a 1e18 amount, a USDC-style 6-decimal substitute). Asserts on
    state — no rescue outbound on the UTX, no new PendingOutbounds row — with the state assertions
    placed before require.Error so a regression fails on state rather than aborting the subtest.
  • A legitimate rescue whose event.PRC20 matches still succeeds and carries the original asset,
    the original PRC20 and the original amount.
  • An original asset with no registered token config produces a clear error and creates nothing.
  • The PRC20 comparison ignores address casing (justifies the canonical comparison).

setupRescueFundsTest previously made the deposit fail by using an unregistered asset, and every
rescue subtest then named an unrelated USDC PRC20 — i.e. the existing suite exercised exactly the
cross-asset shape this fix forbids. It has been corrected, not weakened: the stuck asset is now
registered (18-decimal pETH) and the deposit still fails because its PRC20 has no deployed contract,
which is closer to how a deposit actually fails in production. Subtests built on the USDC-based
bridge/CEA setups name the USDC PRC20 that belongs to their inbound.

Mutation-verified. Reverting the derivation (back to trusting event.PRC20) makes the cross-asset
test fail on the state assertion, with the created outbound showing the substitution directly:

rescue_funds_test.go:256: Expected nil, but got: &types.OutboundTx{
    Amount:"1000000000000000000",                              // the 1e18 pETH deposit
    ExternalAssetAddr:"0x0000000000000000000000000000000000000e07",  // substituted USDC
    Prc20AssetAddr:"0x0000000000000000000000000000000000000e06", ...}
  Messages: no rescue outbound may be created when the event names another asset

…e event

Reject a rescue whose event PRC20 disagrees with the original asset's registered PRC20.
@0xNilesh
0xNilesh merged commit aa5ad0e into audit-fixes Aug 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant