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

`createTaker(...)` does not update `tradeTax` of the offer being taken

audinarey

Summary

The createTaker(...)function does not update the tradetaxfor a given offer

Vulnerability Details

As shown below, this breaks accounting for offers in the protocol as third party protocols may use this value for other purposes.

function createTaker(address _offer, uint256 _points) external payable {
/**
* @dev offer must be virgin
* @dev points must be greater than 0
* @dev total points must be greater than used points plus _points
*/
if (_points == 0x0) {
revert Errors.AmountIsZero();
}
.......
uint256 tradeTax = depositAmount.mulDiv(
makerInfo.eachTradeTax,
Constants.EACH_TRADE_TAX_DECIMAL_SCALER // 10_000
);
// @audit the tradeTax calculation is done but is never updated for the offer

Impact

At the moment this breaks accounting for offers as protocols integrating with Taddle may use it for other purposes

Tools Used

Manual review

Recommendations

Modify the create createTaker(...)to include update for tradeTax of all offers

function createTaker(address _offer, uint256 _points) external payable {
/**
* @dev offer must be virgin
* @dev points must be greater than 0
* @dev total points must be greater than used points plus _points
*/
if (_points == 0x0) {
revert Errors.AmountIsZero();
}
.......
uint256 tradeTax = depositAmount.mulDiv(
makerInfo.eachTradeTax,
Constants.EACH_TRADE_TAX_DECIMAL_SCALER // 10_000
);
+ offerInfo.tradeTax += tradeTax;
Updates

Lead Judging Commences

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

finding-PreMarkets-createTaker-offerInfo-tradetax-settledCollateralAmount-not-updated

Borderline low/informational, tradeTax is posted by the original maker when creating an offer within the `makerInfo` mapping as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L112C13-L112C25). and is simply a placeholder for the offerInfo mapping. Given it can impact details of offer regarding the tradeTax, low severity seems appropriate. Similar reasonings apply for settledCollateralAmount.

Support

FAQs

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

Give us feedback!