Fractional v2 contest - sorrynotsorry's results

Lines of code

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

Vulnerability details

Impact

The Vault.sol contract is provided on the attack vector surfaces where an actor can everytime frontrun it by monitoring the mempool. The init() function that sets the owner of the vault contract can be called by anyone who monitors the mempool. A malicious attacker could monitor the blockchain for bytecode that matches the Vault contract or the init function signature and frontrun the transaction to gain ownership of the contract. This can be repeated as a Denial Of Service (DOS) type of attack, effectively preventing contract deployment and contract ownership, leading to failure of the project design and unrecoverable gas expenses.

Same applies to VaultFactory.sol's deployFor() function since it imports the Vault.sol contract. Once the Vault address is determined as in the code below, the transaction can be frontrun and the deployed vault's owner would be different than the intended address in parameter.

Proof of Concept

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

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

In VaultFactor.sol;

    function deployFor(address _owner) public returns (address payable vault) {
        bytes32 seed = nextSeeds[tx.origin];


        // Prevent front-running the salt by hashing the concatenation of tx.origin and the user-provided seed.
        bytes32 salt = keccak256(abi.encode(tx.origin, seed));


        bytes memory data = abi.encodePacked();
        vault = implementation.clone(salt, data);
        Vault(vault).init();

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/VaultFactory.sol#L62-L70

Tools Used

Manual review

Recommended Mitigation Steps

It should be ensured the init() function is callable only by the deployer/owner of the Vault.sol contract.

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L32 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L38-L41

Vulnerability details

Impact

Vault contract implements receive() and fallback() functions to receive funds. However, there is no method associated with a user to withdraw his funds which might be sent accidentally to a vault contract. This absence of the method results in losing/locking the assets forever in the vault contract. The best example of this kind of occurrence is the WETH and the GOLEM token contracts which have unrecoverable tokens inside.

Proof of Concept

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L32 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L38-L41

Tools Used

Manual review

Recommended Mitigation Steps

The team might consider implementing a withdraw() function.