Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Results of the static analyzer Slither #66

Open
montyly opened this issue May 10, 2019 · 0 comments
Open

Results of the static analyzer Slither #66

montyly opened this issue May 10, 2019 · 0 comments

Comments

@montyly
Copy link

montyly commented May 10, 2019

Hi,

I used the static analyzer Slither on your codebase, and some of the results might interest you.

Let me know if you need more information about these issues.

Dangerous equality

slither . --detect incorrect-equality

Description

An attacker might be able to prevent an address to become a manager by sending cToken to it:
https://github.com/Betoken/betoken/blob/develop/eth/contracts/BetokenLogic.sol#L531-L532

Recommendation

Consider reviewing the logic preventing an address to register again.

Reentrancy

slither . --detect reentrancy-eth,reentrancy-no-eth,reentrancy-benign

Description

Slither reports a lot of potential reentrancy. A thorough analysis of the code is required to determine if they are exploitable.

Note that even if a function is protected through using the nonReentrant, a reentrancy might still be exploited by re-entering to a function not protected by nonReentrant.

Recommendation

Consider using the check-after-effect pattern when possible (https://solidity.readthedocs.io/en/latest/security-considerations.html#re-entrancy).

Slither can be used to ensure that all the functions are protected with nonReentrant (I can help writing the script).

Unused return

slither . --detect unused-return

Description

There are several calls that should check the return value, but does not. The most dangerous seems to be https://github.com/Betoken/betoken/blob/develop/eth/contracts/BetokenFund.sol#L710

Recommendation

Consider checking the success of these calls.

The full list

MiniMeToken.claimTokens (tokens/minime/MiniMeToken.sol#496-506) ignores return value by external calls "token.transfer(owner(),balance)" (tokens/minime/MiniMeToken.sol#504)
LongCERC20OrderLogic.executeOrder (derivatives/LongCERC20OrderLogic.sol#6-47) ignores return value by external calls "COMPTROLLER.enterMarkets(markets)" (derivatives/LongCERC20OrderLogic.sol#26)
ShortCERC20OrderLogic.executeOrder (derivatives/ShortCERC20OrderLogic.sol#6-46) ignores return value by external calls "COMPTROLLER.enterMarkets(markets)" (derivatives/ShortCERC20OrderLogic.sol#25)
BetokenFund.transferAssetToNextVersion (BetokenFund.sol#352-359) ignores return value by external calls "token.transfer(nextVersion,token.balanceOf(address(this)))" (BetokenFund.sol#357)
BetokenFund.withdrawDAI (BetokenFund.sol#673-685) ignores return value by external calls "dai.transfer(msg.sender,_amountInDAI)" (BetokenFund.sol#681)
BetokenFund.withdrawToken (BetokenFund.sol#692-714) ignores return value by external calls "token.transfer(msg.sender,actualTokenWithdrawn)" (BetokenFund.sol#710)
BetokenFund.redeemCommission (BetokenFund.sol#719-736) ignores return value by external calls "dai.transfer(msg.sender,commission)" (BetokenFund.sol#734)
BetokenFund.redeemCommissionForCycle (BetokenFund.sol#744-763) ignores return value by external calls "dai.transfer(msg.sender,commission)" (BetokenFund.sol#761)
BetokenFund.sellLeftoverToken (BetokenFund.sol#769-778) ignores return value by external calls "dai.transfer(devFundingAccount,actualDAIReceived)" (BetokenFund.sol#777)
BetokenFund.sellLeftoverCompoundOrder (BetokenFund.sol#784-797) ignores return value by external calls "dai.transfer(devFundingAccount,outputAmount)" (BetokenFund.sol#796)
BetokenFund.burnDeadman (BetokenFund.sol#803-811) ignores return value by external calls "cToken.destroyTokens(_deadman,cToken.balanceOf(_deadman))" (BetokenFund.sol#810)
BetokenFund.__deposit (BetokenFund.sol#959-967) ignores return value by external calls "sToken.generateTokens(msg.sender,_depositDAIAmount)" (BetokenFund.sol#962)
BetokenFund.__deposit (BetokenFund.sol#959-967) ignores return value by external calls "sToken.generateTokens(msg.sender,_depositDAIAmount.mul(sToken.totalSupply()).div(totalFundsInDAI))" (BetokenFund.sol#964)
BetokenFund.__withdraw (BetokenFund.sol#973-977) ignores return value by external calls "sToken.destroyTokens(msg.sender,_withdrawDAIAmount.mul(sToken.totalSupply()).div(totalFundsInDAI))" (BetokenFund.sol#975)
BetokenFund.__returnStake (BetokenFund.sol#1049-1052) ignores return value by external calls "cToken.destroyTokens(address(this),_stake)" (BetokenFund.sol#1050)
BetokenFund.__returnStake (BetokenFund.sol#1049-1052) ignores return value by external calls "cToken.generateTokens(msg.sender,_receiveKairoAmount)" (BetokenFund.sol#1051)
Utils.__kyberTrade (Utils.sol#110-152) ignores return value by external calls "_srcToken.approve(KYBER_ADDR,0)" (Utils.sol#129)
Utils.__kyberTrade (Utils.sol#110-152) ignores return value by external calls "_srcToken.approve(KYBER_ADDR,_srcAmount)" (Utils.sol#130)
Utils.__kyberTrade (Utils.sol#110-152) ignores return value by external calls "_srcToken.approve(KYBER_ADDR,0)" (Utils.sol#146)
BetokenLogic.nextPhase (BetokenLogic.sol#354-425) ignores return value by external calls "sToken.generateTokens(devFundingAccount,devFunding)" (BetokenLogic.sol#388)
BetokenLogic.nextPhase (BetokenLogic.sol#354-425) ignores return value by external calls "cToken.generateTokens(msg.sender,NEXT_PHASE_REWARD)" (BetokenLogic.sol#422)
BetokenLogic.__handleInvestment (BetokenLogic.sol#792-836) ignores return value by external calls "dai.approve(token,0)" (BetokenLogic.sol#806)
BetokenLogic.__handleInvestment (BetokenLogic.sol#792-836) ignores return value by external calls "dai.approve(token,_actualSrcAmount)" (BetokenLogic.sol#807)
BetokenLogic.__handleInvestment (BetokenLogic.sol#792-836) ignores return value by external calls "dai.approve(token,0)" (BetokenLogic.sol#809)
BetokenLogic.__returnStake (BetokenLogic.sol#863-866) ignores return value by external calls "cToken.destroyTokens(address(this),_stake)" (BetokenLogic.sol#864)
BetokenLogic.__returnStake (BetokenLogic.sol#863-866) ignores return value by external calls "cToken.generateTokens(msg.sender,_receiveKairoAmount)" (BetokenLogic.sol#865)
LongCEtherOrderLogic.executeOrder (derivatives/LongCEtherOrderLogic.sol#6-42) ignores return value by external calls "COMPTROLLER.enterMarkets(markets)" (derivatives/LongCEtherOrderLogic.sol#25)
ShortCEtherOrderLogic.executeOrder (derivatives/ShortCEtherOrderLogic.sol#6-42) ignores return value by external calls "COMPTROLLER.enterMarkets(markets)" (derivatives/ShortCEtherOrderLogic.sol#24)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant