deth-ctf -- `Factory` Incorrectly Stores Vault Address in Memory**

Severity: Medium

Summary

In the Factoryl contract, the deployVault function is responsible for deploying a new vault contract. However, it incorrectly stores the newly deployed vault address in a memory variable named vaultAddress, which causes the contract to disregard the previously deployed vault address. As a result, the lastDeployed state variable is never updated, leading to potential issues with tracking and managing the deployed vaults.

Vulnerability Details

The deployVault function in Factory attempts to deploy a new vault and store its address. However, the vault address is stored in a memory variable (vaultAddress) and not in the lastDeployed state variable, which is intended to keep track of the most recently deployed vault.

Here’s the relevant portion of the code:

contract Factory {
    error AlreadyDeployed();

    address public lastDeployed; // @audit not set

    function deployVault() external {
        address vaultAddress = computeAddress();

        if (vaultAddress.codehash != bytes32(0)) {
            revert AlreadyDeployed();
        } 

        bytes32 salt = bytes32(uint256(uint160(msg.sender)));
        vaultAddress = address(new Vault{salt: salt}(msg.sender)); // <--- @audit wrong 
    }

    function computeAddress() public view returns (address) {
        bytes32 salt = bytes32(uint256(uint160(msg.sender)));
        return address(uint160(uint256(keccak256(abi.encodePacked(
            bytes1(0xff),
            address(this),
            salt,
            keccak256(type(Vault).creationCode)
        )))));
    }
}

The vaultAddress variable is defined within the function scope, which means it is a memory variable and not persistent. Although the vault is successfully deployed, the contract fails to update the lastDeployed state variable with the new vault's address.

Impact

Tracking Deployed Vaults: The lastDeployed state variable is not updated, making it difficult to track the most recently deployed vault. This can cause issues if the contract relies on this information for future operations.
Potential Loss of Functionality: If the lastDeployed variable is used in other parts of the contract or system, this bug could result in malfunctioning logic, as it would hold an outdated or incorrect value.

Tools Used

Manual Review

Recommendations

Update lastDeployed: Assign the vaultAddress to the lastDeployed state variable after deploying the vault. This ensures that the contract correctly tracks the most recent vault.

function deployVault() external {
   address vaultAddress = computeAddress();

   if (vaultAddress.codehash != bytes32(0)) {
       revert AlreadyDeployed();
   } 

   bytes32 salt = bytes32(uint256(uint160(msg.sender)));
   vaultAddress = address(new Vault{salt: salt}(msg.sender));

   lastDeployed = vaultAddress; // Update lastDeployed with the correct vault address
}

Conclusion

The incorrect storage of the vault address in a memory variable rather than updating the lastDeployed state variable can lead to tracking and operational issues in the Factory contract. By fixing this oversight, the contract will properly manage the deployment and tracking of vault addresses, ensuring accurate and reliable operations.

Egis-Security / CTF_Challenge