code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

`validateWeights()` Limit loop to a meaningful bound can save gas #131

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Basket.sol#L64-L68

function validateWeights(address[] memory _tokens, uint256[] memory _weights) public override pure {
    require(_tokens.length == _weights.length);
    uint256 length = _tokens.length;
    address[] memory tokenList = new address[](length);

    // check uniqueness of tokens and not token(0)

    for (uint i = 0; i < length; i++) {
        require(_tokens[i] != address(0));
        require(_weights[i] > 0);

        for (uint256 x = 0; x < tokenList.length; x++) {
            require(_tokens[i] != tokenList[x]);
        }

        tokenList[i] = _tokens[i];
    }
}

for (uint256 x = 0; x < tokenList.length; x++) can be change to for (uint256 x = 0; x < i; x++) because the value of tokenList[i] has not been set yet.

GalloDaSballo commented 2 years ago

Disagree with finding as tokenList.length is always equal at most i because of the require