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

Incorrect Validation of ```eachTradeTax``` and ```collateralRate``` in ```PreMarktes::createOffer```

kiteweb3

Summary

The PreMarktes::createOffer contains validation checks for the eachTradeTax and collateralRate parameters. The NatSpec comments indicate that eachTradeTax must be less than 100% and collateralRate must be more than 100%, with a decimal scaler of 10000 (link: https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L42-L43).
However, the actual implementation uses > and < operators, which do not align with the specified requirements.

Vulnerability Details

The current implementation uses the following conditions (https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L49C8-L55C10):

function createOffer(CreateOfferParams calldata params) external payable {
/**
* @dev points and amount must be greater than 0
* @dev eachTradeTax must be less than 100%, decimal scaler is 10000
* @dev collateralRate must be more than 100%, decimal scaler is 10000
*/
if (params.points == 0x0 || params.amount == 0x0) {
revert Errors.AmountIsZero();
}
​
@> if (params.eachTradeTax > Constants.EACH_TRADE_TAX_DECIMAL_SCALER) {
revert InvalidEachTradeTaxRate();
}
​
@> if (params.collateralRate < Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
revert InvalidCollateralRate();
}
... omitted code
}

These conditions allow eachTradeTax to be exactly 100% and collateralRate to be exactly 100%, which contradicts the NatSpec comments.

Impact

This discrepancy can lead to unintended behavior where offers with eachTradeTax equal to 100% and collateralRate equal to 100% are accepted, causing financial losses within the marketplace.

Tools Used

Manual review

Recommendations

Update the validation conditions:

- if (params.eachTradeTax > Constants.EACH_TRADE_TAX_DECIMAL_SCALER) {
+ if (params.eachTradeTax >= Constants.EACH_TRADE_TAX_DECIMAL_SCALER) {
revert InvalidEachTradeTaxRate();
}
​
- if (params.collateralRate < Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
+ if (params.collateralRate <= Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
revert InvalidCollateralRate();
}
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-off-by-one-Collateral-Rate-100%

I believe this is borderline informational/low. Despite this off-by-one error of the intended check, the difference between 100% and 101% is minimal, so I believe whether or not 100% is allowed has minimal impact. Ultimately, it still comes down to the risk level that users are willing to take

finding-PreMarkets-off-by-one-Trade-TAX-100%

Similar to issue #1323, Despite this off-by-one error of the intended check, the difference between 99% and 100% is minimal, so I believe whether or not 100% is allowed has minimal impact. Ultimately, takers should not be realistically creating offer with such tradeTax

Support

FAQs

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