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: 14/39
Findings: 3
Award: $1,220.07
🌟 Selected for report: 2
🚀 Solo Findings: 0
🌟 Selected for report: cccz
Also found by: 0x1f8b, Dravee, TomFrenchBlockchain, UncleGrandpa925, WatchPug, bobi, byterocket, hack3r-0m, sirhashalot
0x1f8b
Unsafe transfers.
In LaunchEvent there are multiple transfers without checking the boolean result, ERC20 standard specify that the token can return false if the transfer was not made, so it's mandatory to check the result of transfer methods.
Affected lines:
Manual review
Check the boolean result.
#0 - cryptofish7
2022-02-11T00:20:01Z
Duplicate of #12
#1 - dmvt
2022-02-22T10:49:41Z
This could result in a loss of funds given the right external conditions.
2 — Med (M): vulns have a risk of 2 and are considered “Medium” severity when assets are not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
0x1f8b
Unsafe approve was done.
In the method LaunchEvent.execute:406-407 are made two approve without checking the boolean result, ERC20 standard specify that the token can return false if the approve was not made, so it's mandatory to check the result of approve methods.
Manual review
Use safe approve or check the boolean result
#0 - cryptofish7
2022-02-11T00:22:31Z
Duplicate of #154
🌟 Selected for report: 0x1f8b
518.9746 USDT - $518.97
0x1f8b
Wrong contract initialization.
During the initialize it was not checked that the contract it's owner of _joe, it's required to be because it will be called the mint method in line 175, so it's mandatory to ensure that the contract it's the owner.
Manual review.
Check that RocketJoeStaking is the owner of joe
🌟 Selected for report: sirhashalot
Also found by: 0v3rf10w, 0x1f8b, Dravee, UncleGrandpa925, cccz, defsec, gzeon
0x1f8b
Detailed description of the impact of this finding.
Inside the constructor of RocketJoeFactory it's well checked if the provided addresses are equal to address(0), but the owner can change this values around different methods like setPenaltyCollector or setRouter, setFactory.. and inside this methods this checks are not present, it's mandatory to do the same checks in all the methods in order to ensure the expected values.
Also the method setPhaseDuration must throw an exception if the _phaseNumber it`s not 1 or 2.
Manual review.
Check the provided arguments.
#0 - cryptofish7
2022-02-11T00:23:48Z
Duplicate of #263
🌟 Selected for report: 0x1f8b
518.9746 USDT - $518.97
0x1f8b
Owner can Denial of service.
In the contract RocketJoeStaking there are two ways to set rJoePerSec, one in the initialize and the second one in updateEmissionRate, in both of them there are no checks of the received value, so it's possible to use a high value and deny the service in line updatePool:168.
Manual review
Change the type to uint128 for rJoePerSec.
#0 - cryptofish7
2022-02-11T00:32:22Z
Fix: https://github.com/traderjoe-xyz/rocket-joe/pull/112
Should be 1 however.
#1 - dmvt
2022-02-23T13:02:45Z
While I agree with the warden that this would represent a denial of service attack, the likelihood that the admin would do this to their own staking pool is very low, making this a low risk.
0x1f8b
Gas saving.
In the contract RocketJoeFactory the variables eventImplementation and wavax can be immutable in order to save gas also there are two codding style issues that can be applied.
The following public arguments are in uppercase, usually this style is used for constant, and can be confused for developers, it's recommended to change to camel case.
uint256 public override PHASE_ONE_DURATION = 2 days; uint256 public override PHASE_ONE_NO_FEE_DURATION = 1 days; uint256 public override PHASE_TWO_DURATION = 1 days;
Also there are a lot of argument ussing override keyword and it's not required, this keyword can be removed.
Manual review.
Apply the suggested changes.
#0 - cryptofish7
2022-02-11T00:26:29Z
Duplicate of #284
17.9007 USDT - $17.90
0x1f8b
Gas saving.
There are a unneeded cast of IJoeFactory(IJoeFactory) in line LaunchEvent:384 and RocketJoeFactory:123, in this last one it's required to store the factory as IJoeFactory instead of address.
Manual review
Remove unneeded cast.
#0 - cryptofish7
2022-02-11T00:23:10Z
Duplicate of #235
17.9007 USDT - $17.90
0x1f8b
Gas saving.
In the contract RocketJoeStaking It's possible to save gas changing the storage keyword to memory in pendingRJoe.user variable, because it won't be changed, so it's cheaper to use memory.
Gas saving
Apply the mentioned changes.
#0 - cryptofish7
2022-02-11T00:33:32Z
Duplicate of #116