sherlock-audit / 2024-03-arrakis-judging

0 stars 0 forks source link

RadCet - Users lost their private vault #87

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

RadCet

medium

Users lost their private vault

Summary

In ArrakisMetaVaultFactory.sol, there is a function name deployPrivateVault for user to create their private vault, however attacker make users can't track their private vault

Vulnerability Detail

Function is deployPrivateVault using create3 with parameter vault = Create3.create3(salt, creationCode). The vault address will be determined by the salt value and the creationCode. Salt value will be calculated by: bytes32 salt = keccak256(abi.encode(msg.sender, salt_)). Both msg.sender and salt_ can be observed by an attacker when TX is mined. The creationCode is calculated like this:

bytes memory creationCode = abi.encodePacked(
            ICreationCode(creationCodePrivateVault).getCreationCode(),
            abi.encode(
                moduleRegistryPrivate,
                manager,
                token0_,
                token1_,
                address(nft)
            )
        );

Attacker can have all initialize parameter's value here (public variable and from TX), can get Initialization Code of private vault by calling getCreationCode in arrakis-modular\src\CreationCodePrivateVault.sol. With all that information, an attacker can frontrun the creation TX with their own creation request (can use create2 opcode directly), with the same parameters. When the victim's transaction would be executed, the address is non-empty so the EVM would reject its creation. This would result in a bad UX for a user, cause they saw that the creation was not success. The result vault would still be usable, but would be hard to track as it was created in another TX.

Impact

As mention above, bad UX for user, and it hard for user to track their private vault. Furthermore, the NFT can't be mined for user, the result vault would not be added to _privateVaults array. Can consider it like a DOS attack. Likelihood: not easy but possible Impact: medium So I think this will be a medium issue

Code Snippet

ArrakisMetaVaultFactory.sol

        bytes32 salt = keccak256(abi.encode(msg.sender, salt_));

        // #endregion compute salt = salt + msg.sender.

        // #region get the creation code for TokenMetaVault.

        bytes memory creationCode = abi.encodePacked(
            ICreationCode(creationCodePrivateVault).getCreationCode(),
            abi.encode(
                moduleRegistryPrivate,
                manager,
                token0_,
                token1_,
                address(nft)
            )
        );

        // #endregion get the creation code for TokenMetaVault.

        vault = Create3.create3(salt, creationCode);
        nft.mint(owner_, uint256(uint160(vault)));

Tool used

Manual Review

Recommendation

Use an private ever-increasing nonce counter to guarantee unique contract addresses.