code-423n4 / 2024-01-salty-findings

4 stars 3 forks source link

Timelock Bypass #1012

Closed c4-bot-9 closed 5 months ago

c4-bot-9 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/ManagedWallet.sol#L65

Vulnerability details

Impact

Detailed description of the impact of this finding.

Timelock Bypass: There is no check to reset the activeTimelock if a proposal is rejected. If the confirmationWallet confirms a proposal and then sends less than 0.05 ETH before the changeWallets function is called, the activeTimelock would be set to type(uint256).max, effectively bypassing the timelock.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

// Done this way in case custodial wallets are used as the confirmationWallet - which sometimes won't allow for smart contract calls.
    if ( msg.value >= .05 ether )
        activeTimelock = block.timestamp + TIMELOCK_DURATION; // establish the timelock
    else
@>      activeTimelock = type(uint256).max; // effectively never
    }

Tools Used

vs code

Recommended Mitigation Steps

we have to implement timelock in a different way so that our timelock cannot be bypassed.

Assessed type

Context

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Insufficient quality