code/cl8y-bridge-monorepo

Fork 0

security(terra): pending src_decimals must not change payout after approve #180

New issue

Open

opened 2026-09-12 12:29:11 +00:00 by PlasticDigits · 0 comments

PlasticDigits commented

2026-09-12 12:29:11 +00:00

Owner

Summary

Terra V2 AdminFixPendingDecimals rewrites PendingWithdraw.src_decimals on any non-executed row. Execute mint/unlock pays normalize_decimals(amount, src_decimals, dest_decimals) from that stored field. Operator approve attests the deposit hash (amount + accounts + nonce), not the scaled payout. Cancelers watch the same hash. After they have signed off, admin can still change the scale that execute uses.

This is not #175 (WithdrawApprove ignores min_signatures). This is not #177 (timelock / 15s floor on cancel-window config). This is not #135 (wasm config.admin handoff to DEX 2-of-3). Those share admin / approve / delay words. The bug is post-attest src_decimals mutation plus no bind to TOKEN_SRC_MAPPINGS.

Internal review ids: TERRA-H1 and TERRA-H2 (high). Still in source as of 2026-09-12.

Bundle (same ticket, do not split):

Restrict decimal fix to pre-approval, or treat any post-submit decimal change as a new attestation: clear approved, require a fresh approve, reset approved_at / cancel window.
Cap the new src_decimals to the live TOKEN_SRC_MAPPINGS value for that (src_chain, src_token) (not an arbitrary 0..=18).
Execute-time sanity: payout scale must match the mapping (and dest_decimals vs TOKENS.terra_decimals) or revert.
Tests that post-approval inflation (and deflation) via this message is rejected.

Founder-required CosmWasm. No community autoland. Do not add ready.

Impact (today vs hypothetical)

Funds at risk today in source if config.admin is compromised (or already hostile). Not a permissionless drain: non-admin cannot call AdminFixPendingDecimals. After operators approve and cancelers let the window expire, execute still reads mutable pending.src_decimals. Lowering src_decimals relative to dest_decimals multiplies payout (normalize_decimals scales up when src < dest). The hash does not include decimals, so the attested deposit can pay a different Terra amount than cancelers inferred from the mapping at submit.

This is theft/inflation of a pending user withdraw (mint or unlock), not a lock bug. Rate limits may trip on huge scaled amounts; they are not a control. Sticky until CosmWasm is patched (no storage-layout change required for the pre-approval restriction).

Hypothetical-only if every live wasm has never been instantiated with this message, or admin is already 2-of-3 with no single-key path. Source still wires the message and the execute path.

Do not publish a mainnet approve-then-fix-then-execute sequence.

TERRA-H1: any non-executed row, including post-approve

execute_admin_fix_pending_decimals (packages/contracts-terraclassic/bridge/src/execute/withdraw.rs):

info.sender == config.admin else Unauthorized.
src_decimals > 18 rejected.
Load pending; executed → WithdrawAlreadyExecuted.
pending.src_decimals = src_decimals; save; emit old/new.

No check of pending.approved, cancelled, or approved_at. No reset of the cancel window. Natspec says “Only allowed on approved-but-not-executed withdrawals,” which is the dangerous window and is not what the code enforces (unapproved rows are also writable). Either way, post-approval rewrite is allowed.

WithdrawSubmit copies src_mapping.src_decimals and token_config.terra_decimals onto the row. compute_xchain_hash_id hashes src/dest chain, accounts, token, raw amount, nonce — not decimals. execute_withdraw_approve sets approved / approved_at after operator-or-admin auth and nonce checks; it does not re-read mappings or freeze scale.

Cancel: canceler-only, requires approved, within approved_at + WITHDRAW_DELAY. After window_end, cancel is CancelWindowExpired. Decimal fix after that point cannot be cancelled.

Execute unlock and mint both call normalize_decimals(pending.amount, pending.src_decimals, pending.dest_decimals) then rate-limit and pay that payout_amount. load_and_validate_execution checks approved / not cancelled / window passed — not decimals vs mapping.

TERRA-H2: unbounded value, no mapping bind, no execute check

The new src_decimals is any u8 ≤ 18. It is not required to equal TOKEN_SRC_MAPPINGS[(src_chain, src_token)].src_decimals. Admin SetIncomingTokenMapping can change the mapping independently; that does not rewrite existing pending rows (hence this admin message). Execute never reloads the mapping for scale.

test_withdraw_flow.rs covers submit storing mapping decimals and execute with the correct 18→6 (and large-amount) conversion. There is no test of AdminFixPendingDecimals. CI cannot catch post-approval inflation.

Watchtower docs (docs/contracts-terraclassic.md) describe operator approve + delay + canceler verify. They do not mention admin rewriting payout scale after that verify.

Why the new implementation is needed

The watchtower invariant is: after approve, cancelers verify a frozen payout intent. src_decimals is payout intent. Leaving it admin-mutable after approve makes canceler review of the hash incomplete.
A mapping-correction tool is reasonable before approve (wrong TOKEN_SRC_MAPPINGS at submit). After approve it must re-enter the watchtower or be forbidden.
0..=18 without a mapping cap lets a compromised admin pick the scale that maximizes mint/unlock, independent of the registered source decimals.
Tests currently prove the honest decimal path and omit the admin rewrite. That locks in the hole.

Constraints / guardrails

Do not remove admin-only auth on this message. Do not open it to operators or users.
Do not weaken canceler-only cancel, delay, pause, rate limits, (src_chain, nonce) replay, or hash inputs. Do not add decimals into the canonical deposit hash (that would break EVM/Solana parity). Freeze or re-attest pending scale instead.
Prefer pre-approval only (!pending.approved, and reject if cancelled / executed). Alternative: on any decimal change, set approved = false, approved_at = 0, keep the row in ACTIVE_WITHDRAW_HASHES (INV-TC-AW1), require a new WithdrawApprove (which restarts approved_at). Pick one; do not leave a silent post-approve write.
Cap: src_decimals must equal the current mapping’s src_decimals (after any intended mapping fix). Do not accept a value that is merely <= 18. If product needs to fix mapping and pending in one flow, require mapping update first, then pending copy of that same value, still pre-approval or with re-approve.
Execute: normalize_decimals must use a scale that matches mapping + TOKENS.terra_decimals (or the frozen snapshot taken at approve). Mismatch → revert, do not pay. Same rule on unlock and mint.
Do not change dest_decimals via this message unless the same pre-approval / re-attest + mapping/TOKENS cap is applied. Today only src_decimals is writable; do not add a second unbounded knob.
PendingWithdraw shape can stay; no required new field if the restriction is “unapproved only” + execute mapping check. If you snapshot scale at approve, use #[serde(default)] for any new field so canonical rows still load.
Operator writer / frontend: out of scope except tests/docs must not claim admin can “fix decimals” on an already-approved hash without a new window.
Founder-required wasm / admin key. No community autoland. Do not add ready. No public mainnet fix recipe.

Relevant files

Path	Why
`packages/contracts-terraclassic/bridge/src/execute/withdraw.rs`	`execute_admin_fix_pending_decimals`; submit copies mapping decimals; approve ignores scale; execute `normalize_decimals`
`packages/contracts-terraclassic/bridge/src/msg.rs`	`ExecuteMsg::AdminFixPendingDecimals`
`packages/contracts-terraclassic/bridge/src/contract.rs`	Dispatches the admin message
`packages/contracts-terraclassic/bridge/src/execute/config.rs`	`SetIncomingTokenMapping` updates mapping `src_decimals` without pending rows
`packages/contracts-terraclassic/bridge/src/state.rs`	`PendingWithdraw.{src,dest}_decimals`; `TOKEN_SRC_MAPPINGS`
`packages/contracts-terraclassic/bridge/tests/test_withdraw_flow.rs`	Honest 18→6 execute; add post-approval reject + mapping-cap cases
`docs/contracts-terraclassic.md`	Watchtower section omits payout-scale freeze

Recommended direction

Pre-approval only (preferred): revert AdminFixPendingDecimals unless !approved && !executed && !cancelled. Copy only TOKEN_SRC_MAPPINGS.src_decimals (argument must equal mapping, or ignore the argument and copy mapping). Unapproved-only keeps cancelers from reviewing a hash whose scale later changes.
Or re-attest: allow the write when not executed, but clear approval and restart the window so cancelers see the new queried src_decimals before execute can succeed.
Execute guard: both mint and unlock reload mapping + token config; if pending.src_decimals != mapping.src_decimals or pending.dest_decimals != token_config.terra_decimals, revert with a dedicated error (e.g. DecimalScaleMismatch). This also catches a mapping edit that was not copied, and a hostile copy that drifted from mapping.
Tests: post-approve fix must fail (or, if re-attest, execute before the new window must fail and cancelers get a new window). Pre-approve fix to the mapping value still works. Arbitrary src_decimals not equal to mapping fails. Existing 18→6 execute stays green.

Acceptance criteria

AC1. After WithdrawApprove, AdminFixPendingDecimals with a different src_decimals reverts (preferred) or clears approved and resets approved_at. Execute must not pay the new scale without a new completed watchtower cycle.
AC2. Before approve, admin may set src_decimals only to the live mapping value. 0..=18 that does not match mapping reverts.
AC3. Execute mint and unlock revert if pending scale ≠ mapping / terra_decimals. Honest submit (no admin fix) still pays the mapping conversion (existing 1e18 → 1e6 uluna fixture).
AC4. Non-admin still Unauthorized. Executed rows still WithdrawAlreadyExecuted. Cancel / delay / pause / nonce behavior unchanged except the chosen re-attest reset.
AC5. Docs: watchtower model states that payout scale is frozen at approve (or re-enters the window). Natspec on execute_admin_fix_pending_decimals must match the shipped restriction (today it describes the unsafe window).
AC6. Focused CosmWasm tests pass (cargo test -p bridge or the repo’s documented Terra contract suite).

Test plan (functional paths)

#	Path	Expect
T1	Submit with mapping 18, query pending	`src_decimals == 18`, `dest_decimals == 6`
T2	Pre-approve `AdminFixPendingDecimals` to mapping 18 (no-op)	Succeeds; still unapproved
T3	Pre-approve fix to mapping after `SetIncomingTokenMapping` corrected 18→6	Pending becomes 6; still unapproved
T4	Pre-approve fix to `0` while mapping is 18	Revert (not mapping)
T5	Approve, then fix to any other `src_decimals`	Revert (AC1 preferred) or approval cleared + window restart
T6	T5 then execute after original window, without new approve	Revert (`WithdrawNotApproved` or window active)
T7	Honest approve + delay + execute (no fix)	Existing 18→6 payout
T8	Non-admin fix	`Unauthorized`
T9	Fix after execute	`WithdrawAlreadyExecuted`
T10	Execute with pending decimals tampered in fixture vs mapping	`DecimalScaleMismatch` (or equivalent)

Test plan (attack, hack, and abuse)

Non-exploitative. cw-multi-test / local only. Do not use these as a mainnet recipe.

#	Vector	Expect
A1	Approve, lower `src_decimals` so `normalize_decimals` scales up, wait original window, execute	Fix rejected or new window not expired; payout not inflated
A2	Approve, raise `src_decimals` to shrink user payout	Same as A1 (no silent deflation)
A3	Change `TOKEN_SRC_MAPPINGS` after approve without pending fix, execute	Revert on execute sanity (do not pay new mapping)
A4	Fix during cancel window without resetting `approved_at`	Forbidden; cancelers must see a restarted window if re-attest is chosen
A5	Fix on cancelled-but-not-executed row then uncancel	Either fix rejected, or uncancel still starts a full new window on the original mapping scale
A6	`src_decimals = 0` (≤18) with dest 6	Rejected unless mapping is actually 0
A7	Admin as both mapping editor and fixer post-approve	Still cannot skip watchtower (AC1)

Verification criteria

cargo test -p bridge (or documented Terra suite): T1–T10 and A1–A6 in test_withdraw_flow.rs (or a dedicated decimals test module).
Grep: execute_admin_fix_pending_decimals checks approved / mapping equality (or the documented re-attest). Execute mint and unlock both compare scale to mapping before normalize_decimals payout.
Docs/natspec match shipped behavior. No “approved-but-not-executed” as the allowed fix window unless re-attest is implemented and tested.
Do not verify by broadcasting AdminFixPendingDecimals on columbus-5.

Out of scope

#175 N-of-M approve votes.
#177 cancel-window timelock / 15s floor (this ticket may reset approved_at; it must not retarget delay governance).
#135 admin 2-of-3 handoff. A later 2-of-3 admin does not replace a payout-scale freeze.
EVM/Solana decimal handling; Terra RecoverAsset / LOCKED_BALANCES.
Operator/canceler off-chain monitors (they should keep reading queried src_decimals, but the contract must not require them to win a race after the window).
Live wasm migrate / key rotation (ops).

First-pass model recommendation

Recommendation: grok-high

Rationale: Security class plus founder-required CosmWasm (withdraw.rs payout scale, PendingWithdraw attestation vs execute, mapping bind, watchtower window). Composer is disallowed (High/security; contracts / wasm / admin keys / wallet 2-of-3). Not a local three-file tweak: execute mint and unlock, mapping cap, tests, natspec/docs. A wrong allow (post-approve write without resetting approved_at) leaves canceler review of the hash meaningless. Verify with multi-test approve-then-fix fixtures, not a live columbus-5 execute.

## Summary Terra V2 `AdminFixPendingDecimals` rewrites `PendingWithdraw.src_decimals` on any non-executed row. Execute mint/unlock pays `normalize_decimals(amount, src_decimals, dest_decimals)` from that stored field. Operator approve attests the deposit hash (amount + accounts + nonce), not the scaled payout. Cancelers watch the same hash. After they have signed off, admin can still change the scale that execute uses. This is not [#175](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/175) (WithdrawApprove ignores `min_signatures`). This is not [#177](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/177) (timelock / 15s floor on cancel-window *config*). This is not [#135](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/135) (wasm `config.admin` handoff to DEX 2-of-3). Those share admin / approve / delay words. The bug is post-attest `src_decimals` mutation plus no bind to `TOKEN_SRC_MAPPINGS`. Internal review ids: TERRA-H1 and TERRA-H2 (high). Still in source as of 2026-09-12. Bundle (same ticket, do not split): 1. Restrict decimal fix to **pre-approval**, *or* treat any post-submit decimal change as a new attestation: clear `approved`, require a fresh approve, reset `approved_at` / cancel window. 2. Cap the new `src_decimals` to the live `TOKEN_SRC_MAPPINGS` value for that `(src_chain, src_token)` (not an arbitrary `0..=18`). 3. Execute-time sanity: payout scale must match the mapping (and `dest_decimals` vs `TOKENS.terra_decimals`) or revert. 4. Tests that post-approval inflation (and deflation) via this message is rejected. Founder-required CosmWasm. No community autoland. Do not add `ready`. ## Impact (today vs hypothetical) Funds at risk today in source if `config.admin` is compromised (or already hostile). Not a permissionless drain: non-admin cannot call `AdminFixPendingDecimals`. After operators approve and cancelers let the window expire, execute still reads **mutable** `pending.src_decimals`. Lowering `src_decimals` relative to `dest_decimals` multiplies payout (`normalize_decimals` scales up when `src < dest`). The hash does not include decimals, so the attested deposit can pay a different Terra amount than cancelers inferred from the mapping at submit. This is theft/inflation of a pending user withdraw (mint or unlock), not a lock bug. Rate limits may trip on huge scaled amounts; they are not a control. Sticky until CosmWasm is patched (no storage-layout change required for the pre-approval restriction). Hypothetical-only if every live wasm has never been instantiated with this message, or admin is already 2-of-3 with no single-key path. Source still wires the message and the execute path. Do not publish a mainnet approve-then-fix-then-execute sequence. ### TERRA-H1: any non-executed row, including post-approve `execute_admin_fix_pending_decimals` (`packages/contracts-terraclassic/bridge/src/execute/withdraw.rs`): - `info.sender == config.admin` else `Unauthorized`. - `src_decimals > 18` rejected. - Load pending; `executed` → `WithdrawAlreadyExecuted`. - `pending.src_decimals = src_decimals`; save; emit old/new. No check of `pending.approved`, `cancelled`, or `approved_at`. No reset of the cancel window. Natspec says “Only allowed on approved-but-not-executed withdrawals,” which is the **dangerous** window and is **not** what the code enforces (unapproved rows are also writable). Either way, post-approval rewrite is allowed. `WithdrawSubmit` copies `src_mapping.src_decimals` and `token_config.terra_decimals` onto the row. `compute_xchain_hash_id` hashes src/dest chain, accounts, token, **raw `amount`**, nonce — not decimals. `execute_withdraw_approve` sets `approved` / `approved_at` after operator-or-admin auth and nonce checks; it does not re-read mappings or freeze scale. Cancel: canceler-only, requires `approved`, within `approved_at + WITHDRAW_DELAY`. After `window_end`, cancel is `CancelWindowExpired`. Decimal fix after that point cannot be cancelled. Execute unlock and mint both call `normalize_decimals(pending.amount, pending.src_decimals, pending.dest_decimals)` then rate-limit and pay that `payout_amount`. `load_and_validate_execution` checks approved / not cancelled / window passed — not decimals vs mapping. ### TERRA-H2: unbounded value, no mapping bind, no execute check The new `src_decimals` is any `u8` ≤ 18. It is **not** required to equal `TOKEN_SRC_MAPPINGS[(src_chain, src_token)].src_decimals`. Admin `SetIncomingTokenMapping` can change the mapping independently; that does **not** rewrite existing pending rows (hence this admin message). Execute never reloads the mapping for scale. `test_withdraw_flow.rs` covers submit storing mapping decimals and execute with the **correct** 18→6 (and large-amount) conversion. There is **no** test of `AdminFixPendingDecimals`. CI cannot catch post-approval inflation. Watchtower docs (`docs/contracts-terraclassic.md`) describe operator approve + delay + canceler verify. They do not mention admin rewriting payout scale after that verify. ## Why the new implementation is needed 1. The watchtower invariant is: after approve, cancelers verify a frozen payout intent. `src_decimals` is payout intent. Leaving it admin-mutable after approve makes canceler review of the hash incomplete. 2. A mapping-correction tool is reasonable **before** approve (wrong `TOKEN_SRC_MAPPINGS` at submit). After approve it must re-enter the watchtower or be forbidden. 3. `0..=18` without a mapping cap lets a compromised admin pick the scale that maximizes mint/unlock, independent of the registered source decimals. 4. Tests currently prove the honest decimal path and omit the admin rewrite. That locks in the hole. ## Constraints / guardrails - Do not remove admin-only auth on this message. Do not open it to operators or users. - Do not weaken canceler-only cancel, delay, pause, rate limits, `(src_chain, nonce)` replay, or hash inputs. Do not add decimals into the canonical deposit hash (that would break EVM/Solana parity). Freeze or re-attest **pending** scale instead. - Prefer **pre-approval only** (`!pending.approved`, and reject if `cancelled` / `executed`). Alternative: on any decimal change, set `approved = false`, `approved_at = 0`, keep the row in `ACTIVE_WITHDRAW_HASHES` (INV-TC-AW1), require a new `WithdrawApprove` (which restarts `approved_at`). Pick one; do not leave a silent post-approve write. - Cap: `src_decimals` must equal the current mapping’s `src_decimals` (after any intended mapping fix). Do not accept a value that is merely `<= 18`. If product needs to fix mapping and pending in one flow, require mapping update first, then pending copy of that same value, still pre-approval or with re-approve. - Execute: `normalize_decimals` must use a scale that matches mapping + `TOKENS.terra_decimals` (or the frozen snapshot taken at approve). Mismatch → revert, do not pay. Same rule on unlock and mint. - Do not change `dest_decimals` via this message unless the same pre-approval / re-attest + mapping/`TOKENS` cap is applied. Today only `src_decimals` is writable; do not add a second unbounded knob. - `PendingWithdraw` shape can stay; no required new field if the restriction is “unapproved only” + execute mapping check. If you snapshot scale at approve, use `#[serde(default)]` for any new field so canonical rows still load. - Operator writer / frontend: out of scope except tests/docs must not claim admin can “fix decimals” on an already-approved hash without a new window. - Founder-required wasm / admin key. No community autoland. Do not add `ready`. No public mainnet fix recipe. ## Relevant files | Path | Why | | --- | --- | | `packages/contracts-terraclassic/bridge/src/execute/withdraw.rs` | `execute_admin_fix_pending_decimals`; submit copies mapping decimals; approve ignores scale; execute `normalize_decimals` | | `packages/contracts-terraclassic/bridge/src/msg.rs` | `ExecuteMsg::AdminFixPendingDecimals` | | `packages/contracts-terraclassic/bridge/src/contract.rs` | Dispatches the admin message | | `packages/contracts-terraclassic/bridge/src/execute/config.rs` | `SetIncomingTokenMapping` updates mapping `src_decimals` without pending rows | | `packages/contracts-terraclassic/bridge/src/state.rs` | `PendingWithdraw.{src,dest}_decimals`; `TOKEN_SRC_MAPPINGS` | | `packages/contracts-terraclassic/bridge/tests/test_withdraw_flow.rs` | Honest 18→6 execute; add post-approval reject + mapping-cap cases | | `docs/contracts-terraclassic.md` | Watchtower section omits payout-scale freeze | ## Recommended direction 1. **Pre-approval only (preferred):** revert `AdminFixPendingDecimals` unless `!approved && !executed && !cancelled`. Copy only `TOKEN_SRC_MAPPINGS.src_decimals` (argument must equal mapping, or ignore the argument and copy mapping). Unapproved-only keeps cancelers from reviewing a hash whose scale later changes. 2. **Or re-attest:** allow the write when not executed, but clear approval and restart the window so cancelers see the new queried `src_decimals` before execute can succeed. 3. **Execute guard:** both mint and unlock reload mapping + token config; if `pending.src_decimals != mapping.src_decimals` or `pending.dest_decimals != token_config.terra_decimals`, revert with a dedicated error (e.g. `DecimalScaleMismatch`). This also catches a mapping edit that was not copied, and a hostile copy that drifted from mapping. 4. Tests: post-approve fix must fail (or, if re-attest, execute before the new window must fail and cancelers get a new window). Pre-approve fix to the mapping value still works. Arbitrary `src_decimals` not equal to mapping fails. Existing 18→6 execute stays green. ## Acceptance criteria - AC1. After `WithdrawApprove`, `AdminFixPendingDecimals` with a different `src_decimals` reverts (preferred) **or** clears `approved` and resets `approved_at`. Execute must not pay the new scale without a new completed watchtower cycle. - AC2. Before approve, admin may set `src_decimals` only to the live mapping value. `0..=18` that does not match mapping reverts. - AC3. Execute mint and unlock revert if pending scale ≠ mapping / `terra_decimals`. Honest submit (no admin fix) still pays the mapping conversion (existing 1e18 → 1e6 uluna fixture). - AC4. Non-admin still `Unauthorized`. Executed rows still `WithdrawAlreadyExecuted`. Cancel / delay / pause / nonce behavior unchanged except the chosen re-attest reset. - AC5. Docs: watchtower model states that payout scale is frozen at approve (or re-enters the window). Natspec on `execute_admin_fix_pending_decimals` must match the shipped restriction (today it describes the unsafe window). - AC6. Focused CosmWasm tests pass (`cargo test -p bridge` or the repo’s documented Terra contract suite). ## Test plan (functional paths) | # | Path | Expect | | --- | --- | --- | | T1 | Submit with mapping 18, query pending | `src_decimals == 18`, `dest_decimals == 6` | | T2 | Pre-approve `AdminFixPendingDecimals` to mapping 18 (no-op) | Succeeds; still unapproved | | T3 | Pre-approve fix to mapping after `SetIncomingTokenMapping` corrected 18→6 | Pending becomes 6; still unapproved | | T4 | Pre-approve fix to `0` while mapping is 18 | Revert (not mapping) | | T5 | Approve, then fix to any other `src_decimals` | Revert (AC1 preferred) or approval cleared + window restart | | T6 | T5 then execute after original window, without new approve | Revert (`WithdrawNotApproved` or window active) | | T7 | Honest approve + delay + execute (no fix) | Existing 18→6 payout | | T8 | Non-admin fix | `Unauthorized` | | T9 | Fix after execute | `WithdrawAlreadyExecuted` | | T10 | Execute with pending decimals tampered in fixture vs mapping | `DecimalScaleMismatch` (or equivalent) | ## Test plan (attack, hack, and abuse) Non-exploitative. cw-multi-test / local only. Do not use these as a mainnet recipe. | # | Vector | Expect | | --- | --- | --- | | A1 | Approve, lower `src_decimals` so `normalize_decimals` scales up, wait original window, execute | Fix rejected **or** new window not expired; payout not inflated | | A2 | Approve, raise `src_decimals` to shrink user payout | Same as A1 (no silent deflation) | | A3 | Change `TOKEN_SRC_MAPPINGS` after approve without pending fix, execute | Revert on execute sanity (do not pay new mapping) | | A4 | Fix during cancel window without resetting `approved_at` | Forbidden; cancelers must see a restarted window if re-attest is chosen | | A5 | Fix on cancelled-but-not-executed row then uncancel | Either fix rejected, or uncancel still starts a full new window on the original mapping scale | | A6 | `src_decimals = 0` (≤18) with dest 6 | Rejected unless mapping is actually 0 | | A7 | Admin as both mapping editor and fixer post-approve | Still cannot skip watchtower (AC1) | ## Verification criteria - `cargo test -p bridge` (or documented Terra suite): T1–T10 and A1–A6 in `test_withdraw_flow.rs` (or a dedicated decimals test module). - Grep: `execute_admin_fix_pending_decimals` checks `approved` / mapping equality (or the documented re-attest). Execute mint and unlock both compare scale to mapping before `normalize_decimals` payout. - Docs/natspec match shipped behavior. No “approved-but-not-executed” as the *allowed* fix window unless re-attest is implemented and tested. - Do not verify by broadcasting `AdminFixPendingDecimals` on columbus-5. ## Out of scope - [#175](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/175) N-of-M approve votes. - [#177](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/177) cancel-window timelock / 15s floor (this ticket may *reset* `approved_at`; it must not retarget delay governance). - [#135](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/135) admin 2-of-3 handoff. A later 2-of-3 admin does not replace a payout-scale freeze. - EVM/Solana decimal handling; Terra `RecoverAsset` / `LOCKED_BALANCES`. - Operator/canceler off-chain monitors (they should keep reading queried `src_decimals`, but the contract must not require them to win a race after the window). - Live wasm migrate / key rotation (ops). ## First-pass model recommendation Recommendation: grok-high Rationale: Security class plus founder-required CosmWasm (`withdraw.rs` payout scale, `PendingWithdraw` attestation vs execute, mapping bind, watchtower window). Composer is disallowed (High/security; contracts / wasm / admin keys / wallet 2-of-3). Not a local three-file tweak: execute mint **and** unlock, mapping cap, tests, natspec/docs. A wrong allow (post-approve write without resetting `approved_at`) leaves canceler review of the hash meaningless. Verify with multi-test approve-then-fix fixtures, not a live columbus-5 execute.

PlasticDigits added the

high-risk

security

labels

2026-09-12 12:29:14 +00:00

PlasticDigits referenced this issue

2026-09-12 12:33:25 +00:00

security(solana): bind withdraw_execute TokenMapping to submit-time src_token #181

PlasticDigits referenced this issue

2026-09-12 12:38:22 +00:00

security(operator): Terra watcher must wait for confirmation depth #182