Tadle
Tadle
DeFiFoundry
27,750 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 over 1 year 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.

Give us feedback!