Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Updating user platform fee rate can lead to unexpected token amount user need to transfer to capital pool

0xmusashi

Summary

The function SystemConfig::updateUserPlatformFeeRateis an owner controlled function.

Assuming the owner is trusted, due to the ways transactions are ordered in the blocks, there is a possible scenario that users have to transfer unexpected token amount to capital pool when they create their taker account by calling PreMarkets::createTakerfunction.

Vulnerability Details

Function SystemConfig::updateUserPlatformFeeRatethat update base platform fee rate for a specific user account.

/**
* @notice Update base platform fee rate
* @param _accountAddress Account address
* @param _platformFeeRate Platform fee rate of user
* @notice Caller must be owner
*/
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);
}

Then when users create their Taker accounts by calling PreMarkets::createTakerfunction, the amount of token they have to deposit into capital pool will be calculated based on their platformFeeRateby calling SystemConfig::getPlatformFeeRatefunction.

function createTaker(address _offer, uint256 _points) external payable {
// ...
uint256 platformFeeRate = systemConfig.getPlatformFeeRate(_msgSender());
...
uint256 platformFee = depositAmount.mulDiv(
platformFeeRate,
Constants.PLATFORM_FEE_DECIMAL_SCALER
);
...
_depositTokenWhenCreateTaker(
platformFee,
depositAmount,
tradeTax,
makerInfo,
offerInfo,
tokenManager
);
...
}

Impact

User A (or a multiple of users) queue up a transaction for creating new Taker account.
The owner queues a transaction to update the platform fee for user A.

User A who transaction get executed after the owners transaction, due to network conditions or miners etc, will have to deposit unexpected amount of token to capital pool.

Tools Used

Manual review.

Recommendations

In the SystemConfig::updateUserPlatformFeeRate function, we should add new check that user's platform fee rate is zero or not. This will help the owner to make sure that the user already created his Taker account before the update platform fee rate transaction.

/**
* @notice Update base platform fee rate
* @param _accountAddress Account address
* @param _platformFeeRate Platform fee rate of user
* @notice Caller must be owner
*/
function updateUserPlatformFeeRate(
address _accountAddress,
uint256 _platformFeeRate
) external onlyOwner {
require(
_platformFeeRate <= Constants.PLATFORM_FEE_DECIMAL_SCALER,
"Invalid platform fee rate"
);
require(userPlatformFeeRate[_accountAddress] != 0, "Not created Taker account"); // add this new check
userPlatformFeeRate[_accountAddress] = _platformFeeRate;
​
emit UpdateUserPlatformFeeRate(_accountAddress, _platformFeeRate);
}
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.