c4-bot-3 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/UserVault.sol#L110

Vulnerability details

Impact

There is a missing onlyOwner in the mint function definition that allows anyone to mint NFTs from this vault. This is not correct as it is an unauthorized minting of NFTs.

Proof of Concept

The flawed code is in

UserVault.sol#L110

    /// @inheritdoc IUserVault
    function mint() external returns (uint256) {
        uint256 _vaultId;
        unchecked {
            _vaultId = ++_nextId;
        }
        _mint(msg.sender, _vaultId);
        return _vaultId;
    }

Moreover, the contract inherits from Owned which means there must be functions restricted via the onlyOwner modifier, which is not used in the whole file.

Recommended Mitigation Steps

    /// @inheritdoc IUserVault
-   function mint() external returns (uint256) {
+   function mint() external returns (uint256) onlyOwner {
        uint256 _vaultId;
        unchecked {
            _vaultId = ++_nextId;
        }
        _mint(msg.sender, _vaultId);
        return _vaultId;
    }

Assessed type

Access Control

c4-judge commented 7 months ago

0xA5DF marked the issue as primary issue

0xA5DF commented 7 months ago

Might be intended design, leaving open for sponsor to comment

0xend commented 7 months ago

Intended design. The idea is that anyone can mint and create a bundle with their own assets. A vault on its own has no value.