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: 6/39
Findings: 3
Award: $2,109.26
🌟 Selected for report: 2
🚀 Solo Findings: 1
🌟 Selected for report: jayjonah8
1556.9237 USDT - $1,556.92
jayjonah8
In LauchEvent.sol the withdrawAVAX() function makes an external call to the msg.sender by way of _safeTransferAVAX. This allows the caller to reenter this and other functions in this and other protocol files. To prevent reentrancy and cross function reentrancy there should be reentrancy guard modifiers placed on the withdrawAVAX() function and any other function that makes external calls to the caller.
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/LaunchEvent.sol#L368
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/LaunchEvent.sol#L370
Manual code review
Add reentrancy guard modifier to withdrawAVAX() function.
#0 - cryptofish7
2022-02-01T00:25:38Z
🌟 Selected for report: jayjonah8
518.9746 USDT - $518.97
jayjonah8
In RocketJoeStaking.sol the pendingRJoe() function uses the total supply of the joe token in the contract itself in its calculations for rJoe rewards. This means the rJoe rewards for all users can be manipulated by an attacker simply sending joe tokens into the RocketJoeStaking.sol contract directly. Anyone should not be able to manipulate the rewards system in such a way.
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/RocketJoeStaking.sol#L83
Manual code review
Should not rely on joe.balanceOf(address(this)) to calculate pending rJoe rewards.
#0 - cryptofish7
2022-01-31T20:02:55Z
Disagree with a severity. Our auditor flagged this as an informational for example
#1 - dmvt
2022-02-21T13:32:48Z
I'm going to consider this one low risk. While there is a hypothetical value loss for the actual stakers, it's substantially less than the value lost by the attacker so the likelihood of this happening is very low.
jayjonah8
In the RocketJoeToken.sol file as well as others, floating pragma statements are used. Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/RocketJoeToken.sol#L3
https://swcregistry.io/docs/SWC-103
Manual code review
Lock the pragma version changing them from "pragma solidity ^0.8.0; " to "pragma solidity 0.8.0;"
#0 - cryptofish7
2022-01-30T21:01:55Z
Duplicate of #181
#1 - dmvt
2022-02-22T14:10:43Z
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.
17.9007 USDT - $17.90
jayjonah8
In RocketJoeStaking.sol the deposit() function does not require the _amount argument to be a number larger than 0. It takes in the _amount argument of joe tokens that will be transferred from the user to the contract itself. It then calls the safeTransferFrom() function and emits a Deposit event at the end. This means safeTransferFrom could still be called if the _amount is 0 which would be useless and the Deposit event would still be emitted even though nothing was actually deposited.
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/RocketJoeStaking.sol#L96
Manual code review
Add this to the deposit() function in RocketJoeStaking.sol: require(_amount > 0, "Amount cannot be 0")
#0 - cryptofish7
2022-01-30T23:48:26Z
Duplicate of #317
#1 - dmvt
2022-02-23T12:07:31Z
duplicate of #238, not a risk of lost funds but a gas savings is available.
jayjonah8
In RocketJoeToken.sol there is an initialize() function that can only be called once and it sets the msg.sender address as the rocketJoeFactory in storage. During deployment of the RocketJoeToken.sol contract an attacker can monitor the blockchain byte code and automatically call the initialize() function immediately before the correct rocketJoeFactory gets a chance to do so. This means the attacker would set himself as the rockerJoeFactory and bypass every function with the onlyRJLaunchEvent modifier by making a fake isRJLaunchEvent() function that simply returns true passing the require check. This means the attacker could then call the burnFrom() function with an arbitrary _from address and _amount burning any amount of tokens from any address they want.
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/RocketJoeToken.sol#L25
https://github.com/code-423n4/2022-01-trader-joe/blob/main/contracts/RocketJoeToken.sol#L47
Manual code review
In RocketJoeToken.sol the initialize() function needs to be called by the correct rocketJoeFactory address in the deployment script so an attacker won't be able to call it first.
#0 - cryptofish7
2022-01-30T21:39:13Z
Duplicate of #8
#1 - dmvt
2022-02-21T12:20:08Z
Assets are not at risk. The worst case is that there is lost gas and the contracts need to be redeployed. Consider creating these contracts and calling their initialize functions in a factory to mitigate the issue if not adding the guard.
1 — Low (L): vulns that have a risk of 1 are considered “Low” severity when assets are not at risk. Includes state handling, function incorrect as to spec, and issues with comments.