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

Maker can set `eachTradeTax` to 100%

baz1ka

Relevant GitHub Links

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

Summary

PreMarktes.createOffer() has a bunch of checks to ensure that created offer is correct. One of them checks whether user-provided eachTradeTax parameter is correct. But instead of checking that this parameter doesn't exceed the maximum value for each trade tax, it checks whether it doesn't exceed 100% which is incorrect.

Impact

Users that willing to interact with this offer would need to pay a much bigger fee than it should be if offer owner wants to set it. Maker can make such huge eachTradeTax in case there is a lot of demand where buy/sell points with this offer or resell points using this initial offer.

Proof of Concept

Recommended Mitigation Steps

Check whether eachTradeTax doesn't exceed EACH_TRADE_TAX_MAXINUM instead of EACH_TRADE_TAX_DECIMAL_SCALER:

diff --git a/src/core/PreMarkets.sol b/src/core/PreMarkets.sol
index 5bdbcbf..c3cbeed 100644
--- a/src/core/PreMarkets.sol
+++ b/src/core/PreMarkets.sol
@@ -46,7 +46,7 @@ contract PreMarktes is PerMarketsStorage, Rescuable, Related, IPerMarkets {
revert Errors.AmountIsZero();
}
- if (params.eachTradeTax > Constants.EACH_TRADE_TAX_DECIMAL_SCALER) {
+ if (params.eachTradeTax > Constants.EACH_TRADE_TAX_MAXINUM) {
revert InvalidEachTradeTaxRate();
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

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

Appeal created

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

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

finding-PreMarkets-tradeTax-over-100%

A maximum tradeTax could be valuable to ensure makers do not abuse the tradeTax mechanism as a form of maker bonus. However, ultimately, it would still be user responsibility to take up offers with reasonable tradeTax. In addition, a maximum is already included in the Constants contract represented by EACH_TRADE_TAX_MAXINUM as seen here https://github.com/Cyfrin/2024-08-tadle/blob/72c93f73a26ec7472868cb509e8b454286810223/src/libraries/Constants.sol#L20

Support

FAQs

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