Limit-order lifecycle parser trusts forgeable contract_address attribute (forged fills/cancels for any pair) #285
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic#285
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: High
Reachability: Permissionless. Any deployed contract can emit the right attributes in its own tx.
Affected: limit-order lifecycle parsing (
indexer/src/indexer/parser.rs).Root cause: the parser treats the non-reserved
contract_addressattribute as authoritative for "which pair emitted this", but any contract can set that attribute to any value.Summary
The parser recovers the emitting contract for a
limit_order_fill/ cancel / placement by scanning backward to the nearest contract-address attribute, and it accepts two keys as that source:_contract_addressandcontract_address(is_wasm_contract_addr_key). The underscore form is runtime-reserved — wasmd stamps it and contracts can't set_-prefixed keys. Butcontract_address(no underscore) is an ordinary custom attribute any contract can emit with any value.So a malicious contract emits, in its own event stream:
wasm_contract_addr_beforefinds the forgedcontract_address(it's nearer than the runtime's real_contract_address), so the parser attributes the forged fill to the victim pair.process_limit_order_fillthen looks the pair up by that address and inserts the fill under the real pair id — there's no check that the recovered address matches the reserved_contract_addressthe runtime actually stamped.Same trick forges cancellations (mark a victim's live order cancelled, hiding it from the API/frontend) and placements. It doesn't touch on-chain state, but it corrupts the fills, order lifecycle, and trader analytics the indexer serves — for any pair, from anyone.
Current codebase
parser.rsis_wasm_contract_addr_key:key == "_contract_address" || key == "contract_address".wasm_contract_addr_before: backward scan to the nearest such key — forged value wins over the reserved one.process_limit_order_fill/ cancel / placement handlers: trustfill.pair_address(the recovered value); no cross-check against the runtime-reserved_contract_address.Recommended direction
_contract_addressfor contract scoping. Dropcontract_address(no underscore), or treat it as data, never as the emitter._contract_addressfor that logical emission; reject mismatches.Acceptance criteria
contract_address = <victim_pair>+action = limit_order_filldoes NOT produce a fill row attributed to the victim pair.Test plan (attack / abuse)
All 3 approved.
Additionally need demonstration of the attack in the codebase, and tests to prevent regression.
Fixed directions 1+2 + the attack demo + regression tests you asked for. Direction 3 (factory provenance) flagged as a follow-up — details below.
Fix (directions 1+2): dropped the no-underscore
contract_addressfrom emitter scoping inparser.rs(wasm_contract_addr+is_wasm_contract_addr_key) — only the wasmd-stamped_contract_addressscopes a lifecycle event now; the no-underscore form is data, never the emitter. Since the recovered address is always the reserved one, that satisfies both "only trust_contract_address" and "recovered == reserved" in one change.Attack demonstrated in-codebase (and proven non-vacuous): added
forged_contract_address_fill_not_attributed_to_victim_pairand..._cancel_...— a malicious emitter stamps real_contract_address=terra1attacker, then forgescontract_address=terra1victimpairbeforeaction, and the test asserts the row scopes to the attacker, NOT the victim. I reverted the predicate to the vulnerable form and confirmed both tests FAIL (the forged value attributes to terra1victimpair) — i.e. the attack reproduces — then restored the fix and they pass. Plusgenuine_fill_with_both_contract_address_keys_attributes_to_pair(no regression on the real dual-key shape).No regression — verified against the real chain: a live v4
limit_order_filltx carries_contract_address=<pair>(runtime, before action) AND the pair's owncontract_address=<pair>convenience attr (after action, same value), so legit events scope correctly. Pre-existing test fixtures that used the no-underscore key as the SOLE scope (modeled a shape the chain never emits) were corrected to_contract_address. Full parser suite 29/29.Direction 3 (factory provenance) — follow-up, flagged: with 1+2 in, a forged event now scopes to the attacker's OWN
_contract_address.process_limit_order_fillthen resolves that address and, if unknown, callsdiscover_new_pair, which adopts ANY contract answering{"pair":{}}(pair_discovery.rs). So an attacker whose contract answers{"pair":{}}self-corrupts their own adopted fake pair — NOT a victim, much lower severity. Closing that meansdiscover_new_pairmust verify the address is in the factory's authoritative pair list, which is the pair-discovery trust work that clusters with #279/#286/#287 (store pairs in Postgres via factory). Recommend tracking it there rather than bolting a factory query into the lifecycle path here.Branch
qa/285-forged-contract-address-parser, MR fork→main (no closing keyword). All AC items about victim-targeting are met; #3 residual is attacker-self-corruption pending the provenance follow-up. @PlasticDigitsmentioned in merge request !744
mentioned in issue #287
mentioned in commit
31d090c12amentioned in merge request !750
Verified the forgery vector is closed at the parser, with the attack demo + regression tests you asked for. Mapping the AC:
AC1 (forged
contract_address = victim_pair+action = limit_order_filldoesn't attribute a fill to the victim): met.is_wasm_contract_addr_key/wasm_contract_addrnow match only the runtime-reserved_contract_address; the no-underscore form is data, never the emitter. The forged fill re-scopes to the attacker's own_contract_address, not the victim. Testforged_contract_address_fill_not_attributed_to_victim_pairpasses and is non-vacuous (reverting the predicate makes it fail).AC2 (forged cancel/placement can't alter a victim's served lifecycle): met. Cancel was already covered (
forged_contract_address_cancel_not_attributed_to_victim_pair). Placement was closed at source (same reserved-only predicate on the placement parse paths) but had no named test — addedforged_contract_address_placement_not_attributed_to_victim_pairin MR !750. Parser forged-attack tests 3/3.AC3 (only the runtime-stamped emitter scopes): met. Every lifecycle path (fills/cancels/placements/claims, interleaved and columnar) goes through the reserved-only helper; grep confirms no production code scopes on the no-underscore key. No-regression test
genuine_fill_with_both_contract_address_keys_attributes_to_pairpasses. Parser suite 29/29.Two things straight: (1) no live positive control — the fresh seed has zero lifecycle rows, so "a real fill attributes to its pair" is unit-proven, not live-proven; (2) direction 3 (factory provenance) is still a follow-up — a forged event now self-corrupts the attacker's OWN discovered pair, which clusters with the pair-trust work in #279/#286/#287, not a victim-targeting hole. The victim-targeting AC are met; pending MR !750, @PlasticDigits good to close once you're happy with the provenance follow-up tracking.
mentioned in commit
223128fbdaNeeds to be live proven.
mentioned in issue #311
marked as related to #311
mentioned in issue #316
mentioned in merge request !775
Heads-up from working #316: the _contract_address scoping from this issue (
e951e61) left one integration test red on main — tests/limit_order_parked_lifecycle.rs::park_event_then_claim_updates_db_and_api_filters. Its wasm_park_tx/wasm_claim_tx fixtures still emit the unreserved contract_address, so post-#285 the park/claim events don't match and the order stays active. The fixture sweep here got the parser unit tests but missed this one. Test-only fix (point the fixtures at _contract_address) in MR !775. Not reopening this — just connecting them.mentioned in commit
2e67269718mentioned in commit
bb8af7da19mentioned in merge request !787
Verification complete (agent:verify)
Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/285
Parser fix on
mainverified. Added live-proof harness in MR !787.Acceptance criteria
contract_address=victim+limit_order_filldoes not attribute fill to victimcargo test --lib "forged_contract_address|genuine_fill_with_both_contract_address"(4 tests)forged_contract_address_cancel_not_attributed_to_victim_pair,forged_contract_address_placement_not_attributed_to_victim_pair_contract_addressscopes lifecycle eventsis_wasm_contract_addr_key→_contract_addressonly;limit_order_parked_lifecycleintegrationmake verify-issue-285live leg: hybrid swap → on-chainlimit_order_fillwith_contract_address=<pair>→ indexerGET /api/v1/pairs/{pair}/limit-fillsrow (txE168DBF8EE7695C5…)Full run:
make start && make wait-healthy && make deploy-local && make verify-issue-285→ 12 passed, 0 failed.MR
Guardrails (verify script, L285 invariant doc, agent skill): https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/69
Direction 3 (factory provenance on discovery) remains follow-up on #279/#286/#287.
mentioned in commit
ce9040cb93mentioned in merge request !790
mentioned in commit
50f4417551mentioned in commit
856f024255mentioned in commit
0961244077mentioned in merge request !799
Verification complete (agent:verify)
Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/285
Parser fix on
mainverified. Live positive control confirmed. Guardrails MR: !799 (supersedes conflicted !787).Acceptance criteria
contract_address=victim+limit_order_filldoes not attribute fill to victimcargo test --lib forged_contract_address(3 tests); fill scopes toterra1attacker, notterra1victimpairforged_contract_address_cancel_not_attributed_to_victim_pair,forged_contract_address_placement_not_attributed_to_victim_pair_contract_addressscopes lifecycle eventsis_wasm_contract_addr_key→_contract_addressonly;limit_order_parked_lifecycleintegrationmake verify-issue-285live leg: hybrid swap tx605336BF908325DE…→ LCDlimit_order_fillwith_contract_address=<pair>→ indexerGET /api/v1/pairs/{pair}/limit-fillsrowFull run:
make setup-cloud-localterra && make verify-issue-285→ 12 passed, 0 failed.Parser suite:
cd indexer && cargo test --lib indexer::parser::tests→ 32/32 passed.MR
https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/81 — verify script, L285 invariant doc, agent skill, Cloud Agent
sg dockerfallback for live leg.Direction 3 (factory provenance on discovery) remains follow-up on #279/#286/#287.
mentioned in commit
65c07c1385mentioned in commit
1d644f99d8mentioned in merge request !817
mentioned in commit
fec80e46edmentioned in merge request !823
mentioned in issue #335
mentioned in issue #337
mentioned in merge request !835
mentioned in commit
06eb2d8bb5mentioned in commit
c9fa5ad7damentioned in commit
53b8658c58mentioned in commit
59c1013758mentioned in commit
59c1013758mentioned in commit
8f0ca4009dmentioned in issue #509
mentioned in issue #613
mentioned in issue #614
mentioned in issue #684
mentioned in merge request !1188
mentioned in merge request !1189