Open code423n4 opened 1 year ago
kirk-baird marked the issue as primary issue
kirk-baird marked the issue as selected for report
@kirk-baird Hi, can you help see whether the L-03 mentioned below may be upgraded?
"L-03:getReward() It is recommended to add balance>0 before executing transfer" https://github.com/code-423n4/2023-05-xeth-findings/blob/main/data/bin2chen-Q.md
@kirk-baird Hi, can you help see whether the L-03 mentioned below may be upgraded?
"L-03:getReward() It is recommended to add balance>0 before executing transfer" https://github.com/code-423n4/2023-05-xeth-findings/blob/main/data/bin2chen-Q.md
Yep missed that, I will upgrade L-03 to be a duplicate of this issue.
vaporkane marked the issue as sponsor confirmed
Lines of code
https://github.com/code-423n4/2023-05-xeth/blob/main/src/CVXStaker.sol#L185-L198
Vulnerability details
Zero token transfer can cause a potential DoS in CVXStaker
The CVXStaker contract doesn't check for zero amount while transferring rewards, which can end up blocking the operation.
Impact
The CVXStaker contract is in charge of handling interaction with the Convex pool. The
getReward()
function is used to claim rewards and transfer them to the rewards recipient:https://github.com/code-423n4/2023-05-xeth/blob/main/src/CVXStaker.sol#L185-L198
As we can see in the previous snippet of code, the implementation will loop all the configured reward tokens and transfer them one by one to the reward recipient.
This is a bit concerning as some ERC20 implementations revert on zero value transfers (see https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers). If at least one of the reward tokens includes this behavior, then the current implementation may cause a denial of service, as a zero amount transfer in this token will block the whole action and revert the transaction.
Note that the rewards array is not modifiable, it is defined at construction time, a token cannot be removed.
Proof of concept
We reproduce the issue in the following test.
token1
is a normal ERC20 andtoken2
reverts on zero transfer amounts. Rewards fromtoken1
can't be transferred to the recipient as the zero transfer ontoken2
will revert the operation.Note: the snippet shows only the relevant code for the test. Full test file can be found here.
Recommendation
Check for zero amount before executing the transfer:
Assessed type
ERC20