Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

When the platformFee decreases, users may lose assets.

p0wd3r

Summary

Due to the lack of a refund mechanism for native tokens, users may lose assets when the platformFee decreases.

Vulnerability Details

In createTaker, in addition to the order amount, the user will also transfer in platformFee.

platformFee is retrieved in real time and can be set by the owner.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L829-L836

transferAmount = transferAmount + platformFee + tradeTax;
​
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
);
uint256 platformFeeRate = systemConfig.getPlatformFeeRate(_msgSender());
function updateUserPlatformFeeRate(
address _accountAddress,
uint256 _platformFeeRate
) external onlyOwner {
require(
_platformFeeRate <= Constants.PLATFORM_FEE_DECIMAL_SCALER,
"Invalid platform fee rate"
);
userPlatformFeeRate[_accountAddress] = _platformFeeRate;
​
emit UpdateUserPlatformFeeRate(_accountAddress, _platformFeeRate);
}

In tillIn, it only ensures that msg.value >= amount, but if msg.value > amount, there is no refund.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L79-L90

if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @notice check msg value
* @dev if msg value is less than _amount, revert
* @dev wrap native token and transfer to capital pool
*/
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);

This will result in the following scenario:

  1. Alice expects to deposit 1 ETH through tillIn, msg.value is set to 1 ETH, which includes the platformFee.

  2. In the block where Alice's transaction is located, before Alice's transaction, the owner lowers the platformFee, resulting in only needing to deposit 0.8 ETH.

  3. Due to the lack of a refund mechanism, Alice ends up overpaying by 0.2 ETH.

  4. In this process, neither Alice nor the owner acted maliciously, but it still resulted in Alice's asset loss.

Impact

Users may lose assets when the platformFee decreases.

Tools Used

vscode

Recommendations

Add refund in tillIn

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-Admin-Errors-Malicious

The following issues and its duplicates are invalid as admin errors/input validation/malicious intents are1 generally considered invalid based on [codehawks guidelines](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#findings-that-may-be-invalid). If they deploy/set inputs of the contracts appropriately, there will be no issue. Additionally admins are trusted as noted in READ.ME they can break certain assumption of the code based on their actions, and

Support

FAQs

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

Give us feedback!