code-423n4 / 2021-06-gro-findings

0 stars 1 forks source link

setFeeToken doesn't check index #10

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

gpersoon

Vulnerability details

Impact

The function setFeeToken doesn't check that its parameter index is within bounds. Whereas the comparable function setVault does perform such a check. The risk that something goes wrong is very low though.

Proof of Concept

//https://github.com/code-423n4/2021-06-gro/blob/main/contracts/Controller.sol#L137 function setVault(uint256 index, address vault) external onlyOwner { require(vault != address(0), "setVault: 0x"); require(index < N_COINS, "setVault: !index");

//https://github.com/code-423n4/2021-06-gro/blob/main/contracts/DepositHandler.sol#L70 function setFeeToken(uint256 index) external onlyOwner { address token = ctrl.stablecoins()[index]; require(token != address(0), "setFeeToken: !invalid token");

Tools Used

Recommended Mitigation Steps

Add something like the following to setFeeToken require(index < N_COINS, "setFeeToken: !index");

kitty-the-kat commented 3 years ago

feeToken variable has been removed in favour of hardcoding a solution for usdt.