fix(globe-wallet): enforce timelock on spend limit increases to protect against compromised keys (issue #83) - #98
Closed
s6pa1rta3n-lab wants to merge 1 commit into
Conversation
…ct against compromised keys (issue Orbit-Wal#83)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause Analysis
record_spend's doc comment explicitly states its compromised-key threat model:However,
set_spend_limitpreviously allowed instant limit increases (including raising the limit toi128::MAXor removing it by settinglimit = 0) with onlyuser.require_auth(). Under this threat model, an attacker holding a compromised user key could unilaterally callset_spend_limit(user, asset, i128::MAX)and immediately drain the entire wallet balance past the daily spend cap in a single transaction.Solution & Design Decision
We implement asymmetric handling for spend limit modifications:
current_limit == 0) or lowering an existing cap (limit <= current_limit && limit > 0) is applied instantly viaset_spend_limit. This operation strictly tightens security headroom. Retroactive enforcement is maintained: if today's spend already exceeds the new lower limit, the call is rejected withSpendLimitExceeded.0(unlimited) viaset_spend_limitis rejected withWalletError::SpendLimitIncreaseRequiresProposal. Raising or removing a limit must go through a timelock proposal flow:propose_spend_limit_increase(env, user, asset_code, limit, delay_in_ledgers): Queues the increase proposal with a mandatory timelock delay of at leastGlobeWallet::MIN_SPEND_LIMIT_INCREASE_DELAY(~24 hours / 17,280 ledgers).execute_spend_limit_increase(env, user, asset_code): Executes the proposal once the timelock window has elapsed (env.ledger().sequence() >= ready_at).cancel_spend_limit_increase(env, user, asset_code): Allows the legitimate account owner to notice the unauthorized proposal on-chain and cancel it before execution.get_spend_limit_proposal(env, user, asset_code): Returns the in-flight proposal.Verification & Test Coverage
All tests in the workspace pass (88 tests total). The following tests specifically verify the fix:
test_spend_limit_is_self_escalatable_by_the_key_it_defends_against: Proves that a compromised-key holder cannot instantly raise their own limit and spend past the previously-configured cap in one transaction, and cannot bypass the timelock withdelay = 0or execute before the delay elapses.test_spend_limit_increase_happy_path: Proves legitimate limit increases succeed viapropose_spend_limit_increase-> wait timelock ->execute_spend_limit_increase.test_spend_limit_increase_rejects_delay_below_minimum: Proves proposals withdelay < MIN_SPEND_LIMIT_INCREASE_DELAYare rejected withInvalidTimelockDelay.test_spend_limit_increase_rejects_non_increase: Proves proposals that do not increase the cap are rejected withSpendLimitNotAnIncrease.test_spend_limit_increase_already_pending_fails: Proves second proposals while one is pending are rejected withSpendLimitIncreaseAlreadyPending.test_cancel_spend_limit_increase: Proves proposals can be cancelled and cannot be executed afterward.test_spend_limit_decrease_is_instant_and_unaffected: Proves legitimate limit decreases remain instant.test_raise_spend_then_lower_limit: Verifies retroactive enforcement behavior.Payout Routing
0xF46C9F6d70C50BF81ef3588AB523a90a594a2F89GCL6OXAMLD75BMTINA6EMRUDWK5THQUSHMYNLSNBCJAPZJHNYJTUNIBC