Asymmetry contest - csanuragjain's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L182

Vulnerability details

Impact

If admin mistakenly adds a incorrect derivative contract, then all major contract functionality will permanently fail and User funds will stuck permanently

Proof of Concept

1. Admin wants to add a new derivative using addDerivative function

function addDerivative(
        address _contractAddress,
        uint256 _weight
    ) external onlyOwner {
        derivatives[derivativeCount] = IDerivative(_contractAddress);
        weights[derivativeCount] = _weight;
        derivativeCount++;
        ...
    }

2. Admin wants to use contract address as 0xabc but mistakenly uses 0xabd
3. A new derivative with contract address 0xabd gets created
4. After sometime Admin wants to rebalance weights using rebalanceToWeights which fails since derivatives[i].balance() does not exist for 0xabd contract

function rebalanceToWeights() external onlyOwner {
        uint256 ethAmountBefore = address(this).balance;
        for (uint i = 0; i < derivativeCount; i++) {
            if (derivatives[i].balance() > 0)
                derivatives[i].withdraw(derivatives[i].balance());
        }
...
}

5. Similarly user stake and unstake operation also fails due to same reason

function unstake(uint256 _safEthAmount) external {
        ...

        for (uint256 i = 0; i < derivativeCount; i++) {
            // withdraw a percentage of each asset based on the amount of safETH
            uint256 derivativeAmount = (derivatives[i].balance() *
                _safEthAmount) / safEthTotalSupply;
            if (derivativeAmount == 0) continue; // if derivative empty ignore
            derivatives[i].withdraw(derivativeAmount);
        }

Recommended Mitigation Steps

Allow admin to remove a derivative, which will prevent such scenarios