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

Code inconsistency with the Nat spec

anonymousjoe

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 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!