security(dpp): prove token config update action_id vote swap vulnerability#3345
security(dpp): prove token config update action_id vote swap vulnerability#3345QuantumExplorer wants to merge 1 commit into
Conversation
…ility The action_id for token config updates is derived from only the item TYPE INDEX (u8_item_index), not the actual value. This means: - MaxSupply(100) and MaxSupply(999999999999) produce the SAME action_id - ManualMinting(NoOne) and ManualMinting(ContractOwner) produce the SAME action_id Attack: A group member proposes a conservative config change, gets votes approved, then submits a finalization with a malicious value of the same type. Drive applies the transition payload without verifying it matches the stored group action. Contrast: TokenMintTransition correctly includes the amount in the hash. 4 PoC tests proving the vulnerability, all passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3345 +/- ##
=========================================
Coverage 74.90% 74.90%
=========================================
Files 3000 3000
Lines 271665 271754 +89
=========================================
+ Hits 203478 203564 +86
- Misses 68187 68190 +3
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Well-constructed security PoC demonstrating a real action_id collision vulnerability in TokenConfigUpdateTransition. The 4 tests clearly prove that different config values of the same type produce identical action_ids, enabling vote-swap attacks in multi-party group scenarios. The contrast test against TokenMintTransition effectively shows this is an inconsistency, not a design choice. No production code changes — test-only PR as intended.
Reviewed commit: 5d657f5
|
Yes, it will be fixed. |
|
Closing — this vulnerability is now fixed in PR #3346 which includes the v1 action_id calculation that hashes the full serialized config item payload. The regression test proving the vulnerability exists (and is fixed) is in #3346's test suite ( |
Security Finding: Token Config Update Action ID Vote Swap
Severity: High
Found by: Codex security audit (finding b8a8320f)
Vulnerability
The
action_idfor token configuration updates is computed using only the item type index (u8_item_index()), not the actual value. This means two different config updates of the same type produce identical action IDs:MaxSupply(100)andMaxSupply(999_999_999_999)→ same action_idManualMinting(NoOne)andManualMinting(ContractOwner)→ same action_idAttack Scenario
MaxSupply(100)(conservative limit)hash(... || 4)where 4 = MaxSupply type indexMaxSupply(999_999_999_999)— same action_id since type index is still 4Root Cause
v0_methods.rs:83—update_token_configuration_item.u8_item_index()strips all value informationtoken_config_update_transition.rs:79-85— Drive appliesupdate_token_configuration_itemfrom the submitted transition, not from the stored group actionContrast
TokenMintTransitioncorrectly includesmint_amountin the action_id hash.TokenBurnTransitioncorrectly includesburn_amount. OnlyTokenConfigUpdateTransitionis vulnerable.This PR
4 proof-of-concept tests demonstrating the vulnerability. No fix included — fix should be in a separate PR.
Recommended Fix
TokenConfigurationChangeItemin the action_id hash (not just the type index)