Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/VaultImplementation.sol#L104-L117

Vulnerability details

Impact

The strategist has full control over all key functions. If the strategist is compromised or hacked he will be able to manipulate the vault. For example, increase the depositCap modifyDepositCap(uint256 newCap), add a depositor to the whitelist modifyAllowList(address depositor, bool enabled), or disable the whitelist disableAllowList() and anyone can become a depositor or even shutdown vault. This will break the strategist's intended mechanics and can attract victims to the vault under his control and lead to the loss or freezing of their deposits.

Proof of Concept

file: VaultImplementation.sol

function modifyDepositCap(uint256 newCap) external {
    require(msg.sender == owner()); //owner is "strategist"
    ...
}

function modifyAllowList(address depositor, bool enabled) external virtual {
    require(msg.sender == owner()); //owner is "strategist"
        ...
}

function disableAllowList() external virtual {
    require(msg.sender == owner()); //owner is "strategist"
        ...
}

function enableAllowList() external virtual {
    require(msg.sender == owner()); //owner is "strategist"
    ...
}

function shutdown() external {
    require(msg.sender == owner()); //owner is "strategist"
    ...
}

function setDelegate(address delegate_) external {
    require(msg.sender == owner()); //owner is "strategist"
    ...
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider using an role-based access control approach instead of a single admin role for public vault.

code-423n4 / 2023-01-astaria-findings