Sherlock contest - eriksal1217's results

Handle

eriksal1217

Vulnerability details

Impact

Medium Risk vulnerability - This does not immediately affect the contract, tokens, or funds associated but could have negative effects in regards to how the contract behaves when executing this functionality.

Proof of Concept

According to Slither Analyzer documentaiton (https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return), the return value of an external function call should be used to the benefit of the contract. There should be a reason why the external functions are called in the function and their values should be stored and/or used.

In this case, there are two functions in which the return values are ignored and not used. The first function in question is the LibSherX.accureUSDPool(), although this external function is called, there is no variable that stores this value nor is the value of this function used in the rest of the contract. As of now, it has no effect to the contract and the value is not used, wasting memory and gas.

The second function in question is LibPool.stake(psSherX,withdrawable_amount,from). This function although it returns an unassigned integer, nothing is done with that value in the contract. This function should be checked to make sure that something needs too be done with that value in the contract.

---

Code Snippet:

SherX.redeem(uint256,address) (contracts/facets/SherX.sol, lines #265-295)

ignores return value by:

LibSherX.accrueUSDPool() (contracts/facets/SherX.sol, lines #270)

SherX.doYield(ILock,address,address,uint256) (contracts/facets/SherX.sol, lines #309-358)

ignores return value by:

LibPool.stake(psSherX,withdrawable_amount,from) (contracts/facets/SherX.sol, lines #344)

---

Console Output:

Gov.tokenUnload(IERC20,IRemove,address) (contracts/facets/Gov.sol#271-327) ignores return value by _token.approve(address(_native),totalToken) (contracts/facets/Gov.sol#300) SherX.redeem(uint256,address) (contracts/facets/SherX.sol#265-295) ignores return value by LibSherX.accrueUSDPool() (contracts/facets/SherX.sol#270) SherX.doYield(ILock,address,address,uint256) (contracts/facets/SherX.sol#309-358) ignores return value by LibPool.stake(psSherX,withdrawable_amount,from) (contracts/facets/SherX.sol#344) AaveV2.constructor(IAToken,address,address) (contracts/strategies/AaveV2.sol#39-53) ignores return value by want.approve(address(lp),uint256(- 1)) (contracts/strategies/AaveV2.sol#52) AaveV2.withdraw(uint256) (contracts/strategies/AaveV2.sol#91-96) ignores return value by lp.withdraw(address(want),_amount,msg.sender) (contracts/strategies/AaveV2.sol#95) AaveV2.claimRewards() (contracts/strategies/AaveV2.sol#98-103) ignores return value by aaveIncentivesController.claimRewards(assets,uint256(- 1),aaveLmReceiver) (contracts/strategies/AaveV2.sol#102) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return

Tools Used

Sherlock Contracts Solidity (v 0.7.4) Hardhat (v 2.5.0) Slither Analyzer (v 0.8.0)

Recommended Mitigation Steps

1. Clone repository for Sherlock Smart Contracts
2. Create a python virtual environment with a stable python version
3. Install Slither Analyzer on the python VEM
4. Run Slither against all contracts

Handle

eriksal1217

Vulnerability details

Impact

Gas Optimization - This finding does not affect the contract negatively nor does it put funds or transactions at risk for the user. Optimizes the contract to use less gas

Proof of Concept

According to the Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external), public functions that are never called by the contract itself should be declared as external to save gas. This allows the contract to read directly from the call data. Allows for gas savings if you change a function from public to external, as in the external functions you do not have the extra logic to copy the data to memory.

---

Code snippet:

deposit() should be declared external: - function deposit() public override {...} <------ Should be declared as external

(contracts/strategies/AaveV2.sol#75-81)

---

Console output (Slither Analyzer):

getUnactivatedStakersPoolBalance(IERC20) should be declared external: - PoolBase.getUnactivatedStakersPoolBalance(IERC20) (contracts/facets/PoolBase.sol#146-148)

getTotalUnmintedSherX(IERC20) should be declared external: - PoolBase.getTotalUnmintedSherX(IERC20) (contracts/facets/PoolBase.sol#170-173)

accruedDebt(bytes32,IERC20) should be declared external: - LibPool.accruedDebt(bytes32,IERC20) (contracts/libraries/LibPool.sol#31-34)

getTotalAccruedDebt(IERC20) should be declared external: - LibPool.getTotalAccruedDebt(IERC20) (contracts/libraries/LibPool.sol#36-39)

accrueSherX(IERC20) should be declared external: - LibSherX.accrueSherX(IERC20) (contracts/libraries/LibSherX.sol#75-81)

accrueSherXWatsons() should be declared external: - LibSherX.accrueSherXWatsons() (contracts/libraries/LibSherX.sol#83-86)

deposit() should be declared external: - AaveV2.deposit() (contracts/strategies/AaveV2.sol#75-81)

Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external

---

Tools Used

Sherlock Contracts Solidity (v 0.7.4) Slither Analyzer (v 0.8.0)

Recommended Mitigation Steps

1. Clone repository for Sherlock Smart Contracts
2. Create a python virtual environment with a stable python version
3. Install Slither Analyzer on the python VEM
4. Run Slither against all contracts

Handle

eriksal1217

Vulnerability details

Impact

Medium Risk vulnerability - This does not immediately affect the contract, tokens, or funds associated but could have negative effects in regards to how the contract behaves when executing this functionality.

Proof of Concept

According to Slither Analyzer documentaiton (https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return), the return value of an external function call should be used to the benefit of the contract. There should be a reason why the external functions are called in the function and their values should be stored and/or used.

The following external functions need to be checked to make sure that the values being returned from the external contracts are needed in the internal contract of AaveV2.sol. If they are not required in the contract, the code should be deleted as they use gas and memory in the contract.

---

Code Snippet:

AaveV2.constructor(IAToken,address,address) (contracts/strategies/AaveV2.sol, lines #39-53)

ignores return value by:

want.approve(address(lp),uint256(- 1)) (contracts/strategies/AaveV2.sol, lines #52)

AaveV2.withdraw(uint256) (contracts/strategies/AaveV2.sol, lines #91-96)

ignores return value by:

lp.withdraw(address(want),_amount,msg.sender) (contracts/strategies/AaveV2.sol, lines #95)

AaveV2.claimRewards() (contracts/strategies/AaveV2.sol#98-103)

ignores return value by:

aaveIncentivesController.claimRewards(assets,uint256(- 1),aaveLmReceiver) (contracts/strategies/AaveV2.sol#102)

---

Console output:

Gov.tokenUnload(IERC20,IRemove,address) (contracts/facets/Gov.sol#271-327) ignores return value by _token.approve(address(_native),totalToken) (contracts/facets/Gov.sol#300) SherX.redeem(uint256,address) (contracts/facets/SherX.sol#265-295) ignores return value by LibSherX.accrueUSDPool() (contracts/facets/SherX.sol#270) SherX.doYield(ILock,address,address,uint256) (contracts/facets/SherX.sol#309-358) ignores return value by LibPool.stake(psSherX,withdrawable_amount,from) (contracts/facets/SherX.sol#344) AaveV2.constructor(IAToken,address,address) (contracts/strategies/AaveV2.sol#39-53) ignores return value by want.approve(address(lp),uint256(- 1)) (contracts/strategies/AaveV2.sol#52) AaveV2.withdraw(uint256) (contracts/strategies/AaveV2.sol#91-96) ignores return value by lp.withdraw(address(want),_amount,msg.sender) (contracts/strategies/AaveV2.sol#95) AaveV2.claimRewards() (contracts/strategies/AaveV2.sol#98-103) ignores return value by aaveIncentivesController.claimRewards(assets,uint256(- 1),aaveLmReceiver) (contracts/strategies/AaveV2.sol#102) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return

Tools Used

Sherlock Contracts Solidity (v 0.7.4) Hardhat (v 2.5.0) Slither Analyzer (v 0.8.0)

Recommended Mitigation Steps

1. Clone repository for Sherlock Smart Contracts
2. Create a python virtual environment with a stable python version
3. Install Slither Analyzer on the python VEM
4. Run Slither against all contracts