search

code-423n4 / 2021-07-sherlock-findings

0 stars 0 forks source link

[Gov.sol] Ignoring the return value of function _token.approve(...) #77

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

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, the boolean value for _token.approve(...) should be stored or used to see if the spending of the tokens is allowed and no revert happens during the approval. If this is not checked then it is possible that although the tokens were not approved to be spent, the function could still execute regardless of approval.

Code Snippet:

tokenUnload(IERC20,IRemove,address) (contracts/facets/Gov.sol, lines #271-327)

ignores return value by:

_token.approve(address(_native),totalToken) (contracts/facets/Gov.sol, lines #300)

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
Evert0x commented 3 years ago

51

ghoul-sol commented 3 years ago

Duplicate of #51 and I don't see a reasoning that would justify a medium risk.