Fractional v2 contest - reassor's results

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L125-L132 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L24-L29

Vulnerability details

Impact

Contract Vault implements logic in _execute function to prevent plugins from changing Vault's owner.

    (..)
    // Save the owner address in memory to ensure that it cannot be modified during the DELEGATECALL
    address owner_ = owner;
    // Reserve some gas to ensure that the function has enough to finish the execution
    uint256 stipend = gasleft() - MIN_GAS_RESERVE;

    // Delegate call to the target contract
    (success, response) = _target.delegatecall{gas: stipend}(_data);
    if (owner_ != owner) revert OwnerChanged(owner_, owner);
    (..)

If owner changes then the transaction reverts with OwnerChanged(owner_, owner);.

The issue is that this can be easily bypassed by changing Vault's nonce to 0 and then executing init function to take over the ownership of Vault contract.

    function init() external {
        if (nonce != 0) revert Initialized(owner, msg.sender, nonce);
        nonce = 1;
        owner = msg.sender;
        emit TransferOwnership(address(0), msg.sender);
    }

Exploitation Scenario:

1. Plugin changes nonce to 0
2. Attacker executes init function to set himself as owner
3. Vault's owner has changed

Proof of Concept

Vault.sol:

• https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L125-L132
• https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L24-L29

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to check if nonce has changed and revert if it did.