Platform: Code4rena
Start Date: 25/01/2022
Pot Size: $50,000 USDT
Total HM: 17
Participants: 39
Period: 3 days
Judge: LSDan
Total Solo HM: 9
Id: 79
League: ETH
Rank: 30/39
Findings: 3
Award: $117.20
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: cccz
Also found by: 0x1f8b, Dravee, TomFrenchBlockchain, UncleGrandpa925, WatchPug, bobi, byterocket, hack3r-0m, sirhashalot
bobi
There are some transferFrom() and transfer() calls without checking the results(eg. against reverting). Moreover, for certain ERC20 tokens, should insufficient tokens be present, no revert occurs whatsoever, and instead, a "false" value is returned, which should definitely be checked. It’s important to check for this since doing otherwise would technically mean that you are skipping over a transfer() error, which is, by all means, dangerous and will result in funds lost.
Transfers can be found at:
LaunchEvent.sol::461 => token.transfer(msg.sender, amount); LaunchEvent.sol::467 => pair.transfer(msg.sender, balance); LaunchEvent.sol::493 => token.transfer(msg.sender, amount); LaunchEvent.sol::517 => token.transfer(issuer, balance); LaunchEvent.sol::543 => token.transfer(penaltyCollector, excessToken); LaunchEvent.sol::549 => WAVAX.transfer(penaltyCollector, excessWavax); RocketJoeFactory.sol::133 => IERC20(_token).transferFrom(msg.sender, launchEvent, _tokenAmount); RocketJoeStaking.sol::195 => rJoe.transfer(_to, rJoeBal); RocketJoeStaking.sol::197 => rJoe.transfer(_to, _amount);
Manual analysis.
Best mitigation is to consider using OpenZeppelin's SafeERC20 library with safe versions of transfer functions.
#0 - cryptofish7
2022-02-11T00:48:03Z
Duplicate of #12
bobi
From versions 4.x of ERC20 the approve() call returns a boolean which indicates whether the call has been successful or not.
There are two unchecked approve() calls in your code base, both located at:
LaunchEvent.sol::410 => WAVAX.approve(address(router), wavaxReserve); LaunchEvent.sol::411 => token.approve(address(router), tokenAllocated);
Manual analysis
Although on way would be checking the return value of the approve() call, the best mitigation is to consider using OpenZeppelin's SafeERC20 library with safe versions of safeIncreaseAllowance() / safeDecreaseAllowance() wherever possible, as recommended in the official documentation.
#0 - cryptofish7
2022-02-10T20:06:09Z
Duplicate of #154
bobi
Avoid floating pragmas for non-library contracts.
It is recommended to only use floating pragmas on libraries since they may be used with different versions of solidity applications. It is unsafe, however, to use such pragmas on normal contracts; the reason why is that a vulnerable compiler version might be accidentally selected, thus deploying a different EVM compilation on the blockchain, which would, in turn, put the entire codebase in jeopardy.
Contracts that use a floating pragma :
LaunchEvent.sol => pragma solidity ^0.8.0; RocketJoeFactory.sol => pragma solidity ^0.8.0; RocketJoeStaking.sol => pragma solidity ^0.8.0; RocketJoeToken.sol => pragma solidity ^0.8.0;
Manual analysis
Change to a specific pragma version, eg pragma solidity 0.8.9;
#0 - cryptofish7
2022-02-10T21:54:50Z
Duplicate of #181
#1 - dmvt
2022-02-22T14:09:58Z
Typically I'd consider this a non-critical issue, but in this case I'm going to call it a gas issue given the savings available by locking in a higher version of solidity.
bobi
Defining a variable as immutable, wherever possible, saves gas and increases code clarity.
The following variables could be defined as immutable:
RocketJoeFactory.sol:21: address public override eventImplementation; RocketJoeFactory.sol:25: address public override wavax;
#0 - cryptofish7
2022-02-10T20:02:10Z
Duplicate of #284