Y2k Finance contest - fatherOfBlocks's results

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/rewards/StakingRewards.sol#L34 https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/rewards/StakingRewards.sol#L213 https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/rewards/StakingRewards.sol#L218

Vulnerability details

Impact

• L34/213/218 - In the recoverERC20 function there is a require that is used so that the owner cannot withdraw the staked tokens, but there is no impediment so that he cannot withdraw the rewards tokens, this is a problem since the owner could gradually steal the rewards of the users.

Recommended Mitigation Steps

Add a validation on line 218 that validates that you cannot withdraw tokens from rewardsToken.

Controller.sol

• L93/211 - Less gas cost is generated by doing if(bool) or if(!bool) than by doing bool == true or bool == false, this generates a double validation.

• L200/206 - VaultFactory.getVaults(marketIndex) is executed twice, once to obtain the length and once to obtain only the getter. Much less gas could be generated by doing a single query bringing it into memory to the getter and then on line 200 using the length of the variable in memory.

• L283 - as we try to subtract the execution time of the tx, minus the startedAt of the oracle, that value will always be less, therefore it could become unchecked so that an extra validation of underflows is not executed.

PegOracle.sol

• L29/36 - priceFeed1.decimals() is executed twice. Much less gas could be generated by doing a single query bringing it into memory to the uint and then on lines 29 and 36 using the value in memory.

**SemiFungibleVault.sol **

• L118 - The message shown in the require has a size greater than 32 bytes, this generates an extra gas cost that could be avoided by having a message less than 32 characters.

Vault.sol

• L80/96/217/314 - Less gas cost is generated by doing if(bool) or if(!bool) than by doing bool == true or bool == false, this generates a double validation.

• L187 - It is less expensive to validate that uint(number) != 0 than uint(number) > 0

• L227 - since we try to subtract entitledShares - feeValue and we know that feeValue is a derivative of entitledShares, we can be sure that an underflow will not occur, therefore we can not validate them with unchecked.

• L443 - When a uint256 value is impossible to generate overflow, as for example in a loop for i++, it can be returned unchecked and ++i.

• L443 - When we create a variable and we want it to have its base value, it is not necessary to set that value, since this would imply a double setting.

• L443 - When we are using a for and the length of the array is constantly consulted inside, it would be less expensive to create a variable in memory of the length of the array.

VaultFactory.sol

• L159 - When we create a variable and we want it to have its base value, it is not necessary to set that value, since this would imply a double setting.

• L195 - It is less expensive does variable = variable + 1; which variable += 1;

rewards/StakingRewards.sol

• L36 - When we create a variable and we want it to have its base value, it is not necessary to set that value, since this would imply a double setting.

• L119/134 - It is less expensive to validate that uint(number) != 0 than uint(number) > 0

• L228 - The message shown in the require has a size greater than 32 bytes, this generates an extra gas cost that could be avoided by having a message less than 32 characters.

rewards/RewardsFactory.sol

• L96 - Less gas cost is generated by doing if(bool) or if(!bool) than by doing bool == true or bool == false, this generates a double validation.