Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

The `PerpetualVault::_transferToken()` function doesn't handle errors during token transfer.

sparrowblck

[L-01] Summary

The PerpetualVault::_transferToken() function doesn't handle errors during token transfer.

Vulnerability Details

The PerpetualVault contract is using SafeERC20 for IERC20;; however, it doesn't use the SafeERC20::safeTransfer function when transferring collateral token to the recipient or the treasury contract. As a result, if the IERC20::transfer() function returns a false boolean as a result, then the PerpetualVault state would be updated, but the tokens won't actually be transferred.

In the case of the transfer to the depositInfo[depositId].recipient, there is a catch block, which would catch any reverts, but could not handle a false return value. The case is similar for the transfer to the treasury contract - an exception would revert the whole transaction, but a false return statement would not be detected.

Impact

Medium - the totalDepositAmount would be wrongly updated, when the transfer fails, and a GovernanceFeeCollected event would be emitted. Likelihood is relatively low, as it would depend on the used token.

Tools Used

Slither

Recommendations

Always use SafeERC20::safeTransfer when transferring tokens.

In addition, there should be a remediation mechanism in the treasury contract, in order to retry sending of the funds in case the transfers to users fail.

function _transferToken(uint256 depositId, uint256 amount) internal {
uint256 fee;
if (amount > depositInfo[depositId].amount) {
fee = (amount - depositInfo[depositId].amount) * governanceFee / BASIS_POINTS_DIVISOR;
if (fee > 0) {
collateralToken.safeTransfer(treasury, fee);
}
}
- try collateralToken.transfer(depositInfo[depositId].recipient, amount - fee) {}
+ try collateralToken.safeTransfer(depositInfo[depositId].recipient, amount - fee) {}
catch {
- collateralToken.transfer(treasury, amount - fee);
+ collateralToken.safeTransfer(treasury, amount - fee);
emit TokenTranferFailed(depositInfo[depositId].recipient, amount - fee);
}
totalDepositAmount -= depositInfo[depositId].amount;
emit GovernanceFeeCollected(address(collateralToken), fee);
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!