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

Lows

touthang

L-01:

No function for the owner to withdraw the platform fees.

As we can see in the the tokenManager contract there is no function to withdraw the platform fees taken during a trade, there is a function to rescue token using the Rescuable contract's rescue function but the makerInfo.platformFees will never be deducted nor is it safe to directly withdraw the tokens because there is a high chance of accounting mistake considering the amount of makerInfo that will be created.

Create a function that takes in a makerInfo as an argument and allow the owner to withdraw the makerInfo.platformFeesand reset the makerInfo.platformFees.

L-02:

The whenNotPaused and whenPaused modifiers are not used anywhere in the contracts which makes the pause mechanism useless. Therefore even if the admins call pause function it is useless.
Also from the Pausable contract comments:

/**
* @dev Contract module which allows children to implement an emergency stop
* mechanism that can be triggered by an authorized account.
*
* This module is used through inheritance. It will make available the
* modifiers `whenNotPaused` and `whenPaused`, which can be applied to
* the functions of your contract. Note that they will not be pausable by
* simply including this module, only once the modifiers are put in place.
*/

Recommended mitigation: use the given modifiers in the functions you want it to be pausable, otherwise there is no use of inheriting the Pausable contract.

Info-01:

In the closeBidTaker functio, when the points are not settled for the buyer we calculate the collateral amount to give to the buyer;

uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);

But his part where if the offer is virgin, we sent all the collateral to the buyers.
However, this should not be the case and only the else statement part needs to be executed everytime because there can be some amount of collateral that are being unused, and the buyer should only be incentivized with the used collateral(for the points they bought) and never the unused collateral.

} else {
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
}

Anyway, reporting this as an info because the first if statement will never be triggered as the function already makes sure that the offer.status is settled, which means it is not virgin.

if (offerInfo.offerStatus != OfferStatus.Settled) {
revert InvalidOfferStatus();
}

So, simply remove the first if statement.

Updates

Lead Judging Commences

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