Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ManagedWallet.sol#L73

Vulnerability details

Impact: An hacker can attacks with block timestamp manipulation,which can result in assuming that the active timelock has already expired.

Proof of Concept

We can find the following logic process:changeWallets. Attcker can modify block.timestamp ,and make "block.timestamp >= activeTimelock". In that way ,timelock is completed,but it is cheating.

function changeWallets() external { // proposedMainWallet calls the function - to make sure it is a valid address. require( msg.sender == proposedMainWallet, "Invalid sender" ); require( block.timestamp >= activeTimelock, "Timelock not yet completed" );

    // Set the wallets
    mainWallet = proposedMainWallet;
    confirmationWallet = proposedConfirmationWallet;

    emit WalletChange(mainWallet, confirmationWallet);

    // Reset
    activeTimelock = type(uint256).max;
    proposedMainWallet = address(0);
    proposedConfirmationWallet = address(0);
    }
}

Tools Used

vscode foundry

Recommended Mitigation Steps

Don't rely on block.timestamp as condition . And we can use block.number or something else.

Assessed type

Timing

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