fix(rebalancer): validate partner-API responses before signing/transferring

[droplet-rl]

The rebalancer acts on two partner-API responses using its hot wallet with no runtime validation of the response body. Both are hardened here.

bridge.xyz (BridgeApiClient.createTransferRouteEscrowAddress). The source_deposit_instructions.to_address returned by /v0/transfers is used verbatim as the recipient of an on-chain ERC20 transfer from the rebalancer/withdraw hot wallet, and postWithRetry performs no schema validation. Now verifies the response echoes the requested route (payment_rail/currency) and that the escrow address is a well-formed EVM address before returning it, so a corrupted or incorrect response can't send funds to a non-bridge address.

Paxos Transit (resolvePaxosTransitAuthorization). The erc20_approve transaction.encoded was submitted verbatim as calldata, and the eip2612_permit domain/value were passed straight into _signTypedData — both with no validation. Now:

• the approve calldata is decoded and required to be approve(spender, amount) with spender equal to the requested spender and a bounded amount;
• the permit is required to carry the requested verifyingContract/chainId/owner/spender and a bounded value before signing.

This prevents a compromised or misbehaving endpoint from redirecting an approval to an arbitrary spender, targeting an unexpected token, or authorizing an unbounded amount. The order-quote {to,data,value} target allowlist in PaxosTransitBridge/PaxosTransitL2Bridge is left as a separate follow-up.

Build (tsc --build), prettier and eslint pass.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 614b109933

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Validate the returned destination before funding the escrow

When Bridge returns a stale, corrupted, or malicious transfer whose source rail/currency matches the request but whose destination address, currency, or payment rail differs, these assertions pass and the hot wallet funds that transfer's valid escrow address. The funds can then settle to the destination embedded in the response rather than the requested toAddress; validate the returned destination fields and amount against data before returning the escrow.

Useful? React with 👍 / 👎.

[droplet-rl]

Good catch, fixed in d6d8839.

You're right that the source-route check alone was insufficient: funding the escrow commits the transfer to whatever destination that transfer carries, so a response echoing payment_rail/currency on the source side while naming a different destination would still have settled the funds somewhere we never requested. Now validating the full round trip before returning the escrow:

• destination.payment_rail === this.dstNetwork
• destination.currency === dstTokenSymbol
• destination.to_address matches the requested toAddress
• the echoed amount matches normalizedAmount

One subtlety on the recipient comparison: destinations are not always EVM hex (Tron is in NETWORK_NAMES and uses case-sensitive base58), so destinationAddressesMatch only relaxes to a case-insensitive compare when both sides are hex addresses, and otherwise requires exact equality. Amounts are compared numerically rather than as strings so that trailing-zero normalisation (100.0000 vs 100) doesn't spuriously fail.

Covered by eight new cases in test/generic-adapters/BridgeApi.ts exercising the real BridgeApiClient with a stubbed postWithRetry — redirected recipient, wrong destination currency/rail, wrong source route, mismatched amount, malformed escrow, plus the two positive paths.

[chatgpt-codex-connector]

Validate the EIP-712 schema before signing it

When the Paxos endpoint is compromised, checking properties in value is insufficient because _signTypedData signs only fields selected by the API-controlled types, and ethers ignores extra properties. The endpoint can therefore supply another token-supported schema such as an EIP-3009 authorization, include unused matching owner/spender properties to pass these checks, and obtain a signature authorizing a transfer to an attacker; require the exact EIP-2612 Permit field schema and consistent deadline before signing.

Useful? React with 👍 / 👎.

[droplet-rl]

Confirmed and fixed in d6d8839 — this was the most serious of the three.

I verified the mechanism against ethers v5 directly before fixing: given types describing an EIP-3009 TransferWithAuthorization and a value carrying decoy owner/spender properties, _TypedDataEncoder infers TransferWithAuthorization as the primary type, silently ignores the decoy properties, and _signTypedData returns a valid signature over the transfer authorization. The PR's value-only checks passed the whole way through. USDC supports EIP-3009, so this was reachable on the actual mainnet route.

The fix pins the schema rather than inspecting the payload. assertCanonicalEip2612PermitTypes requires that, after stripping the descriptive EIP712Domain entry, types contains exactly one entry named Permit whose fields are exactly owner, spender, value, nonce, deadline in order with the canonical types. Anything else — an extra type that could become the primary type, a renamed field, a changed field type — is rejected before signing.

Also added the deadline consistency check you flagged: permitData.deadline is forwarded verbatim to the order-quote endpoint alongside the signature, so it now has to equal the value.deadline we actually signed, otherwise the submitted permit wouldn't match the signature.

I left domain.name/version unvalidated deliberately — verifyingContract and chainId are the security boundary, and a wrong name/version only produces a permit that fails on-chain rather than one that authorizes something unintended. Happy to tighten if you'd rather fail fast there.

Regression tests in PaxosTransitBridge.ts cover the exact EIP-3009 decoy payload above, a tampered Permit field list, a mismatched deadline, and a permit bound to the wrong token; each asserts nothing was signed or broadcast from the hot wallet.

[chatgpt-codex-connector]

Update the Paxos positive-path test fixtures

Whenever the checked-in Paxos adapter suite runs, the new validators break two existing positive-path tests: the approval fixture supplies 0xapprove, and the permit fixture uses owner/spender addresses that differ from the requested signer and station. Running RELAYER_TEST=true hardhat test --no-compile test/generic-adapters/PaxosTransitBridge.ts produces 10 passing and 2 failing tests at these validation calls, so the fixtures must be encoded with valid matching authorization data.

Useful? React with 👍 / 👎.

[droplet-rl]

Reproduced exactly as described — RELAYER_TEST=true hardhat test --no-compile test/generic-adapters/PaxosTransitBridge.ts gave 10 passing / 2 failing at those two validation calls. Fixed in d6d8839.

• The approval fixture's placeholder 0xapprove is now real calldata built with an approve(address,uint256) interface targeting the station address and MAX_SAFE_ALLOWANCE.
• The permit fixture now derives owner from l1Signer.getAddress() (which is what PaxosTransitBridge passes as userAddress) and spender from the station address, instead of the 0x…0001/0x…0002 placeholders.

Both fixtures moved into shared encodeApprove / buildPermitData helpers so the new negative tests can override single fields without duplicating the whole blob.

That suite is now 18 passing / 0 failing. Across all of test/generic-adapters/, the branch went from 25 passing / 9 failing to 41 passing / 7 failing; the remaining 7 are pre-existing failures on this branch, unrelated to the PR (missing compiled contract artifacts under --no-compile, in Arbitrum/Linea/OpStack/Polygon/USDC/zkSync/AdapterManager). I confirmed that by stashing the changes and re-running against the unmodified PR head.

[droplet-rl]

@codex thanks — all three findings were real and reproducible. Fixed in d6d8839; replies are on each inline thread, summary here.

1. Transfer destination unvalidated (BridgeUtils) — P1, valid. Validating only the source rail/currency left the destination open: funding the escrow commits the transfer to whatever destination that transfer carries, so a response echoing the source route while naming a different destination would still have settled the funds elsewhere. Now validating the full round trip — destination rail, currency and recipient, plus the echoed amount — before returning the escrow. Destination recipients are compared case-insensitively only when both sides are EVM hex, since Tron (also in NETWORK_NAMES) uses case-sensitive base58; amounts are compared numerically so trailing-zero normalisation doesn't spuriously fail.

2. EIP-712 schema unvalidated (PaxosTransitUtils) — P1, valid, and the most serious. I verified the mechanism against ethers v5 before fixing: with types describing an EIP-3009 TransferWithAuthorization and decoy owner/spender properties in value, ethers infers the attacker's primary type, ignores the decoys, and returns a valid signature over the transfer authorization — while every one of the PR's value-only checks passed. USDC supports EIP-3009, so this was reachable on the real mainnet route. The fix pins types to exactly the canonical EIP-2612 Permit field schema before signing rather than inspecting the payload, and adds the deadline-consistency check (the returned deadline is forwarded to the order-quote endpoint alongside the signature, so it must be the one actually signed).

3. Broken positive-path fixtures — valid. Reproduced your exact 10 passing / 2 failing. The 0xapprove placeholder is now real approve(address,uint256) calldata, and the permit fixture derives owner/spender from the signer and station addresses instead of the 0x…0001/0x…0002 placeholders.

Testing. Added 14 regression tests covering each rejection path, including the EIP-3009 schema-confusion payload; the Paxos ones assert nothing was signed or broadcast from the hot wallet once validation fails. test/generic-adapters/ went from 25 passing / 9 failing to 41 passing / 7 failing. The 7 remaining are pre-existing on this branch and unrelated to the PR — missing compiled contract artifacts under --no-compile in the Arbitrum/Linea/OpStack/Polygon/USDC/zkSync/AdapterManager suites — which I confirmed by stashing the changes and re-running against the unmodified PR head. tsc --noEmit, eslint and prettier are clean.

Two judgement calls worth a human eye:

• I left domain.name/version unvalidated. verifyingContract and chainId are the security boundary; a wrong name/version only yields a permit that fails on-chain rather than one authorizing something unintended.
• The new amount check assumes bridge.xyz echoes the requested source amount in amount. That is consistent with how filterInitiatedTransfers already matches deposit.amount against the on-chain transfer value, but if the API ever returns a net-of-fee figure here this would need loosening.

The order-quote target allowlist noted in the original commit message is still a separate follow-up. Please re-review the updated diff.

[droplet-rl]

🔎 View trace

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 614b109933

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[droplet-rl]

@codex this pass reviewed the wrong commit, so I don't think it clears anything — flagging rather than taking it as a green light.

The comment reports Reviewed commit: 614b109933. That's the pre-fix head. It's also the exact commit you flagged three P1s on nine minutes earlier (review at 00:59:53Z, same SHA) — so the same unchanged code now has two opposite verdicts, which is the tell that this run didn't pick up the new diff.

The fix is in d6d883966d, and it is on the PR — I confirmed the branch head is d6d883966dadcaccb183cc1d52566fa0118e0a3c and that gh api .../pulls/3728/commits lists both commits. Your only submitted review on this PR remains the 00:59:53Z one against 614b109933; nothing has evaluated d6d883966d, which is where all of the security-sensitive logic actually lives:

• the EIP-2612 schema pin (assertCanonicalEip2612PermitTypes) that blocks the EIP-3009 signature-confusion vector from your second finding
• the destination/amount round-trip validation in createTransferRouteEscrowAddress
• the 14 new regression tests, including the decoy-payload case

Timeline for reference: review on 614b109 00:59:53Z → fix committed 01:08:36Z → my re-review request 01:09:30Z → this comment 01:11:29Z, still citing 614b109. Likely a race between the branch push and the review trigger snapshotting the head SHA.

No code changes in response to this comment — there's nothing to act on. Re-review d6d883966d when you can. If the next run also lands on 614b109933, that's a connector-side staleness bug rather than something more prompting will fix, and this should get human eyes instead of another re-trigger — the two P1s were real (I reproduced the EIP-3009 signature against ethers v5 before fixing), so the fix shouldn't merge on the strength of an all-clear that was computed against the vulnerable code.

[droplet-rl]

🔎 View trace

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: d6d883966d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[droplet-rl]

Confirmed — this pass reviewed d6d883966d, which is the actual PR head and the commit containing the fixes. That resolves the staleness I flagged on the previous comment; the re-review has now genuinely evaluated the new diff rather than the pre-fix code.

No code changes in response. Recap of where this landed:

• All three original findings were real and reproduced before fixing — including the EIP-3009 signature-confusion vector, which I verified against ethers v5 directly rather than taking on description.
• Fixes are in d6d883966d: EIP-2612 schema pinning before signing, full destination/amount round-trip validation on the bridge transfer response, and repaired positive-path fixtures.
• test/generic-adapters/ went 25 passing / 9 failing → 41 passing / 7 failing, with 14 new regression tests. The 7 remaining failures are pre-existing and unrelated (missing compiled contract artifacts under --no-compile), confirmed by re-running against the unmodified PR head.
• tsc --noEmit, eslint and prettier are clean.

Two judgement calls from earlier still stand and are worth a human eye at merge time, since an automated clean pass doesn't settle either:

1. domain.name/version are deliberately left unvalidated — verifyingContract and chainId are the security boundary, and a mismatched name/version only yields a permit that fails on-chain rather than one authorizing something unintended.
2. The new amount check assumes bridge.xyz echoes the requested source amount in amount. That matches how filterInitiatedTransfers already treats the field, but would need loosening if the API ever returns a net-of-fee figure there.

The order-quote target allowlist noted in the original commit message remains a separate follow-up. Ready for human review.

[droplet-rl]

🔎 View trace