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: 23/39
Findings: 3
Award: $375.10
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: cccz
Also found by: 0x1f8b, Dravee, TomFrenchBlockchain, UncleGrandpa925, WatchPug, bobi, byterocket, hack3r-0m, sirhashalot
UncleGrandpa925
Users' tokens can be stuck inside LaunchEvent if the token doesn't revert on failed transfers.
When users call withdrawIncentives, if for any reasons the token transfer fails & the token doesn't revert but only returns a boolean, the user's incentives will be stuck. This is because once withdrawIncentives is called, user.hasWithdrawnIncentives will be set to true & therefore prevents the user from calling the function a second time.
Only 2 solutions are possible: either the issurer trigger emergency (which will crash the event), or the issurer can call skim & get back the tokens from the PenaltyCollector to manually distribute it to the user.
Use OZ's SafeTransfer
#0 - cryptofish7
2022-02-10T20:31:43Z
Duplicate of #12
#1 - dmvt
2022-02-22T10:50:21Z
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.
🌟 Selected for report: cmichel
Also found by: UncleGrandpa925, WatchPug, harleythedog
UncleGrandpa925
This issue impacts all LaunchEvent, forcing the issuer to write additional contracts to be able to createPair in LaunchEvent else it will always revert.
In LaunchEvent.sol, the function createPair is supposed to be called by anyone at the start of Phase 3 to create the pair & add all the liquidity inside the LaunchEvent to the pool, getting LP back.
On line 411, the action of adding liquidity set the amountAMin & amountBMin to be the equal to wavaxAddress and tokenAllocated respectively. In other words, this forces add all the liquidity inside LaunchEvent to the pool, and will revert if it can't be done.
An attacker can just transfer a non-zero amount of WAVAX (even 1 wei) to the pool preemptively (as the address of the pool is known before Phase 1, long before Phase 3 where createPair is actually called). The attacker then calls sync to force the wavax reserve to be non-zero.
Now that the reserves are non-zero, the router can't simply use the expected path in the Joe Router02's _addLiquidity: if (reserveA == 0 && reserveB == 0) { to dump all the liquidity in, but will have to go with the else part, where it will certainly fail because of the error JoeLibrary: INSUFFICIENT_LIQUIDITY in the quote function.
To prevent this & totally eliminate the chances of front-running, the issuer will have to write an additional contract to call the pool's skim before calling createPair.
skim before calling addLiquidityProposing this as Medium Risk since it's trivial to exploit and cause troubles for the issurer. Not High risk since the fix is reasonably easy even without modifiying the contract.
#0 - cryptofish7
2022-02-10T20:33:15Z
Duplicate of #281
#1 - dmvt
2022-02-22T19:57:18Z
🌟 Selected for report: sirhashalot
Also found by: 0v3rf10w, 0x1f8b, Dravee, UncleGrandpa925, cccz, defsec, gzeon
31.028 USDT - $31.03
UncleGrandpa925
Lack of checks for data passed into onlyOwner functions.
For example:
setPenaltyCollector. This may lead to penalty being silently transferred to address zero in LaunchEvent.setRouter. This may lead to LaunchEvent's interaction with router (in createPool) to permanently revert, forcing emergency.setPhaseDuration, setPhaseOneNoFeeDuration & setRJoePerAvax, opening up the possibility of erroneous values being set in LaunchEvents, possibly causing these events to be aborted & tokens to be redeployed (since there can only be one LaunchEvent for one token).In conclusion, the lack of checks for these important functions may seriously affect the launch of protocols.
Proposing this as Medium after reffering to a few past reports where there weren't sufficient checks for onlyOwner functions may disrupt the availability of the protocol. Considering the fact that the launch of any protocols is a very important event for them, these issues shouldn't happen.
#0 - cryptofish7
2022-02-10T14:10:42Z
Duplicate of #263
#1 - dmvt
2022-02-22T19:53:07Z
While this could impact availability and potential cause some small fund loss, the likelihood of it occurring is very low given that these are all admin level functions. I agree with the sponsor and other wardens who ranked this low risk.