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

1 stars 0 forks source link

Replace `tokenList.length` by existing variable `length` #230

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

hrkrshnn

Vulnerability details

Replace tokenList.length by existing variable length

modified   contracts/contracts/Basket.sol
@@ -61,7 +61,7 @@ contract Basket is IBasket, ERC20Upgradeable {
             require(_tokens[i] != address(0));
             require(_weights[i] > 0);

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

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

The value tokenList.length is read from memory and therefore requires a mload(...) (6 gas for push memory_offset + mload). On the other hand, this value is already available in the stack as length and could just be dup-ed (3 gas). Saves 3 gas for each loop iteration of the interior loop.

GalloDaSballo commented 2 years ago

Thank you for actually explaining the math behind it!!