Platform: Code4rena
Start Date: 15/07/2021
Pot Size: $80,000 USDC
Total HM: 28
Participants: 18
Period: 7 days
Judge: ghoulsol
Total Solo HM: 18
Id: 20
League: ETH
Rank: 12/18
Findings: 3
Award: $839.15
🌟 Selected for report: 3
🚀 Solo Findings: 0
maplesyrup
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.
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" ],
Spartan Contracts Solidity (v 0.8.3) Slither Analyzer (v 0.8.0)
#0 - SamusElderg
2021-07-22T02:52:06Z
Duplicate of #8
🌟 Selected for report: maplesyrup
160 USDC - $160.00
maplesyrup
Gas optimizations Does not affect the contract in any harmful way. Suggestions allow for smart contract gas optimizations.
According to Slither analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant), the variable in contract Utils.sol called "one" or Utils.one can be set to a constant as it is considered a variable that does not change throughout the contract.
Slither Detectors:
constable-states:
Utils.one (contracts/Utils.sol, lines#11) should be constant
Code in contract:
uint public one = 10**18; <---- can be constant as it does not change
Console output (via Slither in JSON format):
"constable-states": [ "Utils.one (contracts/Utils.sol#11) should be constant\n" ],
Spartan Contracts Solidity (v 0.8.3) Slither Analyzer (v 0.8.0)
🌟 Selected for report: maplesyrup
160 USDC - $160.00
maplesyrup
Gas Optimization This does not directly impact the smart contract in anyway besides cost. This is a gas optimization to reduce cost of smart contract.
According to Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external), there are functions in the contract that are never called. These functions should be declared as external in order to save gas.
Slither Detector:
external-function:
purgeDeployer() should be declared external:
BondVault.purgeDeployer() (contracts/BondVault.sol, lines#50-52)
Console output (via Slither in JSON format):
"external-function": [ "purgeDeployer() should be declared external:\n\t- BondVault.purgeDeployer() (contracts/BondVault.sol#50-52)\n", "hasMinority(uint256) should be declared external:\n\t- Dao.hasMinority(uint256) (contracts/Dao.sol#601-610)\n", "ROUTER() should be declared external:\n\t- Dao.ROUTER() (contracts/Dao.sol#615-621)\n", "UTILS() should be declared external:\n\t- Dao.UTILS() (contracts/Dao.sol#624-630)\n", "BONDVAULT() should be declared external:\n\t- Dao.BONDVAULT() (contracts/Dao.sol#633-639)\n", "DAOVAULT() should be declared external:\n\t- Dao.DAOVAULT() (contracts/Dao.sol#642-648)\n", "POOLFACTORY() should be declared external:\n\t- Dao.POOLFACTORY() (contracts/Dao.sol#651-657)\n", "SYNTHFACTORY() should be declared external:\n\t- Dao.SYNTHFACTORY() (contracts/Dao.sol#660-666)\n", "RESERVE() should be declared external:\n\t- Dao.RESERVE() (contracts/Dao.sol#669-675)\n", "SYNTHVAULT() should be declared external:\n\t- Dao.SYNTHVAULT() (contracts/Dao.sol#678-684)\n", "greet() should be declared external:\n\t- Greeter.greet() (contracts/Greeter.sol#15-17)\n", "setGreeting(string) should be declared external:\n\t- Greeter.setGreeting(string) (contracts/Greeter.sol#19-22)\n" ]
Spartan Contracts Solidity (v 0.8.3) Slither Analyzer (v 0.8.0)
🌟 Selected for report: maplesyrup
394.2006 USDC - $394.20
maplesyrup
This is a low risk vulnerability due to the fact that it is possible to lose funds if the Base address is set to a zero address and someone sends funds to this address. As a rule, there should always be checks to make sure that initialized addresses are never a zero address.
According to Slither analysis documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#exploit-scenario-49), there needs to be a zero address checkpoint when initializing a base address in a contract. In the case for BondVault, the constructor initializes a base address. There should be a check to make sure this address is never zero to make sure there is no way to lose funds.
Slither detector:
missing-zero-check:
BondVault.constructor(address)._base (contracts/BondVault.sol#37) lacks a zero-check on :
BASE = _base (contracts/BondVault.sol#38)
Slither output from console (JSON format):
"missing-zero-check": [ "BondVault.constructor(address)._base (contracts/BondVault.sol#37) lacks a zero-check on :\n\t\t- BASE = _base (contracts/BondVault.sol#38)\n", "Dao.constructor(address)._base (contracts/Dao.sol#96) lacks a zero-check on :\n\t\t- BASE = _base (contracts/Dao.sol#97)\n", "DaoVault.constructor(address)._base (contracts/DaoVault.sol#16) lacks a zero-check on :\n\t\t- BASE = _base (contracts/DaoVault.sol#17)\n", "Pool.constructor(address,address)._base (contracts/Pool.sol#43) lacks a zero-check on :\n\t\t- BASE = _base (contracts/Pool.sol#44)\n", "Pool.constructor(address,address)._token (contracts/Pool.sol#43) lacks a zero-check on :\n\t\t- TOKEN = _token (contracts/Pool.sol#45)\n", "Router.constructor(address,address)._base (contracts/Router.sol#29) lacks a zero-check on :\n\t\t- BASE = _base (contracts/Router.sol#30)\n", "Router.constructor(address,address)._wbnb (contracts/Router.sol#29) lacks a zero-check on :\n\t\t- WBNB = _wbnb (contracts/Router.sol#31)\n", "Synth.constructor(address,address)._base (contracts/Synth.sol#36) lacks a zero-check on :\n\t\t- BASE = _base (contracts/Synth.sol#37)\n", "Synth.constructor(address,address)._token (contracts/Synth.sol#36) lacks a zero-check on :\n\t\t- LayerONE = _token (contracts/Synth.sol#38)\n", "Utils.constructor(address)._base (contracts/Utils.sol#13) lacks a zero-check on :\n\t\t- BASE = _base (contracts/Utils.sol#14)\n", "PoolFactory.constructor(address,address)._base (contracts/poolFactory.sol#28) lacks a zero-check on :\n\t\t- BASE = _base (contracts/poolFactory.sol#29)\n", "PoolFactory.constructor(address,address)._wbnb (contracts/poolFactory.sol#28) lacks a zero-check on :\n\t\t- WBNB = _wbnb (contracts/poolFactory.sol#30)\n", "SynthFactory.constructor(address,address)._base (contracts/synthFactory.sol#15) lacks a zero-check on :\n\t\t- BASE = _base (contracts/synthFactory.sol#16)\n", "SynthFactory.constructor(address,address)._wbnb (contracts/synthFactory.sol#15) lacks a zero-check on :\n\t\t- WBNB = _wbnb (contracts/synthFactory.sol#17)\n", "SynthVault.constructor(address)._base (contracts/synthVault.sol#35) lacks a zero-check on :\n\t\t- BASE = _base (contracts/synthVault.sol#36)\n" ],
Spartan Contracts Solidity (v 0.8.3) Slither Analyzer (v 0.8.0)