search

code-423n4 / 2022-01-trader-joe-findings

2 stars 0 forks source link

No slippage check on `deposit()` of `RocketJoeStaking.sol` #226

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

bobi

Vulnerability details

Impact

There is no slippage check on the deposit() function on RocketJoeStaking.sol. This can lead to a potential front-run attack.

Proof of Concept

This issue is similar to one that has been previously found via the audit of Paladin, namely Issue #05 of the Trader Joe report.

Given that RocketJoeStaking.sol#deposit() adds liquidity to the pool, and the amount of rewards is modifiable, there should be a slippage control put in check, such that the code itself won’t be vulnerable to front-run attacks(sandwich attacks). This is especially important in transactions that are very large in volume.

Tools Used

Manual analysis

Recommended Mitigation Steps

One mitigation would be something to follow suite with Paladin’s recommendations on the previously audit code, namely, instead of:

...
user.amount = user.amount + _amount;
user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;
joe.safeTransferFrom(address(msg.sender), address(this), _amount);
...

You could have:

uint256 balanceBefore = joe.balanceOf(address(this));
joe.safeTransferFrom(address(msg.sender), address(this), _amount);
uint256 receivedAmount = joe.balanceOf(address(this)) - balanceBefore;

user.amount = user.amount + receivedAmount;
user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;
cryptofish7 commented 2 years ago

JOE is known to have no transfer tax

dmvt commented 2 years ago

Agree with sponsor. This token is not variable nor is it vulnerable to the issue described. Invalid.