Tadle

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

Code inconsistency with the Nat spec

Summary

In the createOffer function, the eachTradeTax and collateralRate are mentioned to be strictly < 100% and > 100%. But the code doesnt follow this.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L39-L55

/**
* @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();
}

Here the eachTradeTax and collateralRate can be equal to 100% (10,000), which is contrary to what is mentioned in the comments above.

Impact

Collateral being equal to 100% is not ideal.

Tools Used

Manual Review

Recommendations

Add the '=' sign in both the checks

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

Lead Judging Commences

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