Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: low
Valid

Missing Functionality for Maker to Claim Platform Fee

Summary

Missing Functionality for Maker to Claim Platform Fee

Vulnerability Details

There is currently no function in the contract that allows a maker to claim their platformFee. While the MakerInfo struct includes a platformFee field, there is no mechanism provided for the maker to withdraw or claim this fee. This could lead to issues where the maker is unable to retrieve the fees they are entitled to. Makers get platformFeewhen takers interact with the offercreated by makeror relistedoffers created by taker.

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

uint256 remainingPlatformFee = _updateReferralBonus(
platformFee,
depositAmount,
stockAddr,
makerInfo,
referralInfo,
tokenManager
);
makerInfo.platformFee = makerInfo.platformFee + remainingPlatformFee;

Impact

Maker is unable to retrieve the platform fees they are entitled to.

Tools Used

Manual review

Recommendations

Implement a function that allows makers to claim their platformFee. This function should:

  • Check the platformFee balance in the MakerInfo struct.

  • Transfer the fee to the maker’s address.

  • Reset the platformFee in the MakerInfo struct to zero after the transfer.

Example of a possible function:

function claimPlatformFee() external {
MakerInfo storage maker = makerInfoMap[msg.sender];
uint256 fee = maker.platformFee;
require(fee > 0, "No platform fee to claim");
maker.platformFee = 0; // Reset the fee before transferring to avoid reentrancy attacks
payable(msg.sender).transfer(fee); // Transfer the fee to the maker's address
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-platformFee-no-withdraw-functionality

Low severity, this can be done using the `Rescuable.sol` contract. Arguably there is no errors here given the `platformFee` variable can represent the historical fees that the protocol has accumulated and need not be updated when fees are withdrawn. However, I believe a more explicit function can be valuable to be more transparent regarding withdrawals. However, I will leave this issue open for escalation for debates because I can see it as arguably invalid as well, but I see no arguments for it being medium severity since there is an alternative to retrieve platform fees, assuming admins are trusted.

Support

FAQs

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