This is a high priority vulnerability because it definitely affects the way that funds are transferred and sent between the contracts. You want to make sure that you check the boolean value from these transfer functions in order to make sure that the tokens transferred and revert the transaction if transfers fail. Not checking this can result in an inconsistent state for the contract.
Proof of Concept
According to Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-transfer), it is appropriate and a rule to make sure that the external transfer/transferFrom functions are checked to make sure that the tokens transferred correctly, or revert if the transfers fail. If not, then the function will still execute since there is no check making the token useless since the function will execute anyways. It would be a good idea to use SafeERC20 for any possible overflows/underflows that may happen in the token.
BondVault.claimForMember(address,address) (contracts/BondVault.sol#104-117)
ignores return value by iBEP20(_pool).transfer(member,_claimable) (contracts/BondVault.sol#115)
Synth.sol
unchecked-transfer:
Synth._handleTransferIn(address,uint256) (contracts/Synth.sol#201-208)
ignores return value by iBEP20(_token).transferFrom(msg.sender,address(this),_amount) (contracts/Synth.sol#204)
poolFactory.sol
unchecked-transfer:
PoolFactory._handleTransferIn(address,uint256,address) (contracts/poolFactory.sol#109-115)
ignores return value by iBEP20(_token).transferFrom(msg.sender,_pool,_amount) (contracts/poolFactory.sol#112)
Console output(via Slither, JSON format):
"unchecked-transfer": [
"BondVault.claimForMember(address,address) (contracts/BondVault.sol#104-117) ignores return value by iBEP20(_pool).transfer(member,_claimable) (contracts/BondVault.sol#115)\n",
"Dao.moveBASEBalance(address) (contracts/Dao.sol#218-221) ignores return value by iBEP20(BASE).transfer(newDAO,baseBal) (contracts/Dao.sol#220)\n",
"Dao.handleTransferIn(address,uint256) (contracts/Dao.sol#257-273) ignores return value by iBEP20(_token).transferFrom(msg.sender,address(this),_amount) (contracts/Dao.sol#266)\n",
"Pool.removeForMember(address) (contracts/Pool.sol#192-202) ignores return value by iBEP20(BASE).transfer(member,outputBase) (contracts/Pool.sol#198)\n",
"Pool.removeForMember(address) (contracts/Pool.sol#192-202) ignores return value by iBEP20(TOKEN).transfer(member,outputToken) (contracts/Pool.sol#199)\n",
"Pool.swapTo(address,address) (contracts/Pool.sol#211-226) ignores return value by iBEP20(token).transfer(member,outputAmount) (contracts/Pool.sol#224)\n",
"Pool.burnSynth(address,address) (contracts/Pool.sol#245-257) ignores return value by iBEP20(synthIN).transfer(synthIN,_actualInputSynth) (contracts/Pool.sol#250)\n",
"Pool.burnSynth(address,address) (contracts/Pool.sol#245-257) ignores return value by iBEP20(BASE).transfer(member,outputBase) (contracts/Pool.sol#253)\n",
"Router.zapLiquidity(uint256,address,address) (contracts/Router.sol#59-71) ignores return value by iBEP20(fromPool).transferFrom(_member,fromPool,unitsInput) (contracts/Router.sol#65)\n",
"Router.zapLiquidity(uint256,address,address) (contracts/Router.sol#59-71) ignores return value by iBEP20(_fromToken).transfer(fromPool,iBEP20(_fromToken).balanceOf(address(this))) (contracts/Router.sol#67)\n",
"Router.zapLiquidity(uint256,address,address) (contracts/Router.sol#59-71) ignores return value by iBEP20(BASE).transfer(toPool,iBEP20(BASE).balanceOf(address(this))) (contracts/Router.sol#69)\n",
"Router.removeLiquidityExact(uint256,address) (contracts/Router.sol#101-114) ignores return value by iBEP20(_pool).transferFrom(_member,_pool,units) (contracts/Router.sol#104)\n",
"Router.removeLiquiditySingle(uint256,bool,address) (contracts/Router.sol#117-133) ignores return value by iBEP20(_pool).transferFrom(_member,_pool,units) (contracts/Router.sol#121)\n",
"Router.removeLiquiditySingle(uint256,bool,address) (contracts/Router.sol#117-133) ignores return value by iBEP20(_token).transfer(_pool,iBEP20(_token).balanceOf(address(this))) (contracts/Router.sol#126)\n",
"Router.removeLiquiditySingle(uint256,bool,address) (contracts/Router.sol#117-133) ignores return value by iBEP20(BASE).transfer(_pool,iBEP20(BASE).balanceOf(address(this))) (contracts/Router.sol#129)\n",
"Router._handleTransferIn(address,uint256,address) (contracts/Router.sol#197-211) ignores return value by iBEP20(WBNB).transfer(_pool,_amount) (contracts/Router.sol#203)\n",
"Router._handleTransferIn(address,uint256,address) (contracts/Router.sol#197-211) ignores return value by iBEP20(_token).transferFrom(msg.sender,_pool,_amount) (contracts/Router.sol#207)\n",
"Router._handleTransferOut(address,uint256,address) (contracts/Router.sol#214-224) ignores return value by iBEP20(_token).transfer(_recipient,_amount) (contracts/Router.sol#221)\n",
"Router.swapAssetToSynth(uint256,address,address) (contracts/Router.sol#229-241) ignores return value by iBEP20(BASE).transfer(_pool,iBEP20(BASE).balanceOf(address(this))) (contracts/Router.sol#235)\n",
"Router.swapAssetToSynth(uint256,address,address) (contracts/Router.sol#229-241) ignores return value by iBEP20(BASE).transferFrom(msg.sender,_pool,inputAmount) (contracts/Router.sol#237)\n",
"Router.swapSynthToAsset(uint256,address,address) (contracts/Router.sol#244-265) ignores return value by iBEP20(fromSynth).transferFrom(msg.sender,_poolIN,inputAmount) (contracts/Router.sol#249)\n",
"Synth._handleTransferIn(address,uint256) (contracts/Synth.sol#201-208) ignores return value by iBEP20(_token).transferFrom(msg.sender,address(this),_amount) (contracts/Synth.sol#204)\n",
"PoolFactory._handleTransferIn(address,uint256,address) (contracts/poolFactory.sol#109-115) ignores return value by iBEP20(_token).transferFrom(msg.sender,_pool,_amount) (contracts/poolFactory.sol#112)\n"
],
Tools Used
Spartan Contracts
Solidity (v 0.8.3)
Slither Analyzer (v 0.8.0)
Recommended Mitigation Steps
Clone repository for Spartan Smart Contracts
Create a python virtual environment with a stable python version
SamusElderg
commented
3 years ago
Handle
maplesyrup
Vulnerability details
Impact
This is a high priority vulnerability because it definitely affects the way that funds are transferred and sent between the contracts. You want to make sure that you check the boolean value from these transfer functions in order to make sure that the tokens transferred and revert the transaction if transfers fail. Not checking this can result in an inconsistent state for the contract.
Proof of Concept
According to Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-transfer), it is appropriate and a rule to make sure that the external transfer/transferFrom functions are checked to make sure that the tokens transferred correctly, or revert if the transfers fail. If not, then the function will still execute since there is no check making the token useless since the function will execute anyways. It would be a good idea to use SafeERC20 for any possible overflows/underflows that may happen in the token.
This is also stated in https://docs.soliditylang.org/en/v0.8.0/080-breaking-changes.html for solidity version 0.8.0
Slither Detector:
BondVault.sol
unchecked-transfer:
BondVault.claimForMember(address,address) (contracts/BondVault.sol#104-117) ignores return value by iBEP20(_pool).transfer(member,_claimable) (contracts/BondVault.sol#115)
Synth.sol
unchecked-transfer:
Synth._handleTransferIn(address,uint256) (contracts/Synth.sol#201-208) ignores return value by iBEP20(_token).transferFrom(msg.sender,address(this),_amount) (contracts/Synth.sol#204)
poolFactory.sol
unchecked-transfer:
PoolFactory._handleTransferIn(address,uint256,address) (contracts/poolFactory.sol#109-115) ignores return value by iBEP20(_token).transferFrom(msg.sender,_pool,_amount) (contracts/poolFactory.sol#112)
Console output(via Slither, JSON format):
"unchecked-transfer": [ "BondVault.claimForMember(address,address) (contracts/BondVault.sol#104-117) ignores return value by iBEP20(_pool).transfer(member,_claimable) (contracts/BondVault.sol#115)\n", "Dao.moveBASEBalance(address) (contracts/Dao.sol#218-221) ignores return value by iBEP20(BASE).transfer(newDAO,baseBal) (contracts/Dao.sol#220)\n", "Dao.handleTransferIn(address,uint256) (contracts/Dao.sol#257-273) ignores return value by iBEP20(_token).transferFrom(msg.sender,address(this),_amount) (contracts/Dao.sol#266)\n", "Pool.removeForMember(address) (contracts/Pool.sol#192-202) ignores return value by iBEP20(BASE).transfer(member,outputBase) (contracts/Pool.sol#198)\n", "Pool.removeForMember(address) (contracts/Pool.sol#192-202) ignores return value by iBEP20(TOKEN).transfer(member,outputToken) (contracts/Pool.sol#199)\n", "Pool.swapTo(address,address) (contracts/Pool.sol#211-226) ignores return value by iBEP20(token).transfer(member,outputAmount) (contracts/Pool.sol#224)\n", "Pool.burnSynth(address,address) (contracts/Pool.sol#245-257) ignores return value by iBEP20(synthIN).transfer(synthIN,_actualInputSynth) (contracts/Pool.sol#250)\n", "Pool.burnSynth(address,address) (contracts/Pool.sol#245-257) ignores return value by iBEP20(BASE).transfer(member,outputBase) (contracts/Pool.sol#253)\n", "Router.zapLiquidity(uint256,address,address) (contracts/Router.sol#59-71) ignores return value by iBEP20(fromPool).transferFrom(_member,fromPool,unitsInput) (contracts/Router.sol#65)\n", "Router.zapLiquidity(uint256,address,address) (contracts/Router.sol#59-71) ignores return value by iBEP20(_fromToken).transfer(fromPool,iBEP20(_fromToken).balanceOf(address(this))) (contracts/Router.sol#67)\n", "Router.zapLiquidity(uint256,address,address) (contracts/Router.sol#59-71) ignores return value by iBEP20(BASE).transfer(toPool,iBEP20(BASE).balanceOf(address(this))) (contracts/Router.sol#69)\n", "Router.removeLiquidityExact(uint256,address) (contracts/Router.sol#101-114) ignores return value by iBEP20(_pool).transferFrom(_member,_pool,units) (contracts/Router.sol#104)\n", "Router.removeLiquiditySingle(uint256,bool,address) (contracts/Router.sol#117-133) ignores return value by iBEP20(_pool).transferFrom(_member,_pool,units) (contracts/Router.sol#121)\n", "Router.removeLiquiditySingle(uint256,bool,address) (contracts/Router.sol#117-133) ignores return value by iBEP20(_token).transfer(_pool,iBEP20(_token).balanceOf(address(this))) (contracts/Router.sol#126)\n", "Router.removeLiquiditySingle(uint256,bool,address) (contracts/Router.sol#117-133) ignores return value by iBEP20(BASE).transfer(_pool,iBEP20(BASE).balanceOf(address(this))) (contracts/Router.sol#129)\n", "Router._handleTransferIn(address,uint256,address) (contracts/Router.sol#197-211) ignores return value by iBEP20(WBNB).transfer(_pool,_amount) (contracts/Router.sol#203)\n", "Router._handleTransferIn(address,uint256,address) (contracts/Router.sol#197-211) ignores return value by iBEP20(_token).transferFrom(msg.sender,_pool,_amount) (contracts/Router.sol#207)\n", "Router._handleTransferOut(address,uint256,address) (contracts/Router.sol#214-224) ignores return value by iBEP20(_token).transfer(_recipient,_amount) (contracts/Router.sol#221)\n", "Router.swapAssetToSynth(uint256,address,address) (contracts/Router.sol#229-241) ignores return value by iBEP20(BASE).transfer(_pool,iBEP20(BASE).balanceOf(address(this))) (contracts/Router.sol#235)\n", "Router.swapAssetToSynth(uint256,address,address) (contracts/Router.sol#229-241) ignores return value by iBEP20(BASE).transferFrom(msg.sender,_pool,inputAmount) (contracts/Router.sol#237)\n", "Router.swapSynthToAsset(uint256,address,address) (contracts/Router.sol#244-265) ignores return value by iBEP20(fromSynth).transferFrom(msg.sender,_poolIN,inputAmount) (contracts/Router.sol#249)\n", "Synth._handleTransferIn(address,uint256) (contracts/Synth.sol#201-208) ignores return value by iBEP20(_token).transferFrom(msg.sender,address(this),_amount) (contracts/Synth.sol#204)\n", "PoolFactory._handleTransferIn(address,uint256,address) (contracts/poolFactory.sol#109-115) ignores return value by iBEP20(_token).transferFrom(msg.sender,_pool,_amount) (contracts/poolFactory.sol#112)\n" ],
Tools Used
Spartan Contracts Solidity (v 0.8.3) Slither Analyzer (v 0.8.0)
Recommended Mitigation Steps