Basin - bigtone's results

Lines of code

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L31

Vulnerability details

Impact

There's no validation check of the number of tokens/pumps in the init function. This may cause problems for someone to create a clone of the Well contract without using the LibWellConstructor library.

Proof of Concept

File: src/Well.sol#L31
    function init(string memory name, string memory symbol) public initializer {
        __ERC20Permit_init(name);
        __ERC20_init(name, symbol);

        IERC20[] memory _tokens = tokens();
        for (uint256 i; i < _tokens.length - 1; ++i) {
            for (uint256 j = i + 1; j < _tokens.length; ++j) {
                if (_tokens[i] == _tokens[j]) {
                    revert DuplicateTokens(_tokens[i]);
                }
            }
        } // @audit validation check : numberOfTokens == tokens().length, numberOfPumps()==pumps().length,
    }

Tools Used

Manual review

Recommended Mitigation Steps

There's no way to validate the inputs parameter in the Well contract. So it needs to update the boreWell function to ensure that inputs in the Aquifer.sol. https://github.com/code-423n4/2023-07-basin/blob/main/src/Aquifer.sol#L34 Recommend adding details parameter(tokens, wellFunction, pumps) instead of the immutableData parameter in the boreWell and encodePacked with those details.

Assessed type

Invalid Validation

Gas Optimizations

<a name="GAS-1"></a>[GAS-1] reserves[i] == 0 doesn't need to check

Impact:

It's already checked if reserves[i] is zero in the update function before calling _init() , so it doesn't need to check it.

Vulnerability Detail

File: src/pumps/MultiFlowPump.sol:L153
    function _init(bytes32 slot, uint40 lastTimestamp, uint256[] memory reserves) internal {
        uint256 numberOfReserves = reserves.length;
        bytes16[] memory byteReserves = new bytes16[](numberOfReserves);

        // Skip {_capReserve} since we have no prior reference

        for (uint256 i; i < numberOfReserves; ++i) {
            if (reserves[i] == 0) return; // @audit it doesn't need to check
            byteReserves[i] = reserves[i].fromUIntToLog2();
        }

Recommendation

Recommend removing the line that checks if reserves[i] == 0