Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

[M-2] `PreMarkets::_updateTokenBalanceWhenCreateTaker` sends trade tax to the reseller instead of the original offer maker

philbugcatcher

Summary

When a maker creates an offer and sets a tax rate, the intention is for the maker to take a share of the future negotiations involving the points they have listed. However, the current implementation incorrectly sends the collected taxes to the reseller instead of the maker.

Vulnerability Details

The issue stems from the logic in PreMarkets::_updateTokenBalanceWhenCreateTaker, which does not correctly handle the tax distribution for offers marked with OfferSettleType.Protected. The current implementation mistakenly directs the trade tax to the reseller when it should be directed to the maker, as shown below:

Vulnerability Details
function _updateTokenBalanceWhenCreateTaker(
address _offer,
uint256 _tradeTax,
uint256 _depositAmount,
OfferInfo storage offerInfo,
MakerInfo storage makerInfo,
ITokenManager tokenManager
) internal {
if (
_offer == makerInfo.originOffer ||
makerInfo.offerSettleType == OfferSettleType.Protected
) {
tokenManager.addTokenBalance(
TokenBalanceType.TaxIncome,
@> offerInfo.authority,
makerInfo.tokenAddress,
_tradeTax
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.TaxIncome,
makerInfo.authority,
makerInfo.tokenAddress,
_tradeTax
);
}
​
/// @dev update sales revenue
if (offerInfo.offerType == OfferType.Ask) {
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
offerInfo.authority,
makerInfo.tokenAddress,
_depositAmount
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
_depositAmount
);
}
}

Impact

This vulnerability leads to unexpected financial losses by the makers as the taxes intended for them are misallocated to resellers, who will in turn receive unintended financial benefits.

The impact is demonstrated in the test case below, which can be included in the PreMarkets.t.sol file:

Proof Of Code
function testResellerGetsTradeTax() public {
address maker = vm.addr(777); // maker that will not receive the expected trade tax
address reseller = vm.addr(778); // reseller that will incorrectly receive trade tax
address buyer = vm.addr(779); // buyer created to simulate trade tax being applied
​
// set up of the demonstration
uint256 STARTING_BALANCE = 100e18;
uint256 POINTS_AMOUNT = 1_000;
uint256 TOKEN_AMOUNT = 1e18;
uint256 TOKEN_AMOUNT_RESELL = 2e18;
uint256 COLLATERAL_RATE = 12_000;
uint256 EACH_TRADE_TAX = 1_000; // maker decides for a 10% trade tax
​
deal(address(mockUSDCToken), maker, STARTING_BALANCE);
deal(address(mockUSDCToken), reseller, STARTING_BALANCE);
deal(address(mockUSDCToken), buyer, STARTING_BALANCE);
​
vm.prank(maker);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.prank(reseller);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.prank(buyer);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
vm.prank(address(capitalPool));
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
// maker creates an offer
vm.prank(maker);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
POINTS_AMOUNT,
TOKEN_AMOUNT,
COLLATERAL_RATE,
EACH_TRADE_TAX,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
address offer0Addr = GenerateAddress.generateOfferAddress(0);
​
// reseller buys original offer and then lists is for sale
vm.startPrank(reseller);
preMarktes.createTaker(offer0Addr, POINTS_AMOUNT);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, TOKEN_AMOUNT_RESELL, COLLATERAL_RATE);
address offer1Addr = GenerateAddress.generateOfferAddress(1);
vm.stopPrank();
​
// buyer buys secondary offer (created by reseller)
vm.prank(address(buyer));
preMarktes.createTaker(offer1Addr, POINTS_AMOUNT);
​
// reseller gets the tax that was supposed to go to the maker
uint256 resellerBalanceBeforeTaxIncome = mockUSDCToken.balanceOf(
reseller
);
vm.prank(reseller);
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.TaxIncome
);
uint256 resellerBalanceAfterTaxIncome = mockUSDCToken.balanceOf(
reseller
);
​
assert(
resellerBalanceAfterTaxIncome - resellerBalanceBeforeTaxIncome > 0
);
}

Tools Used

Manual code review.

Recommendations

To avoid this issue, adjust the function PreMarkets::_updateTokenBalanceWhenCreateTaker to send the tax income to the maker instead of the offer authority:

- if (
- _offer == makerInfo.originOffer ||
- makerInfo.offerSettleType == OfferSettleType.Protected
- ) {
- tokenManager.addTokenBalance(
- TokenBalanceType.TaxIncome,
- offerInfo.authority,
- makerInfo.tokenAddress,
- _tradeTax
- );
- } else {
tokenManager.addTokenBalance(
TokenBalanceType.TaxIncome,
makerInfo.authority,
makerInfo.tokenAddress,
_tradeTax
);
- }
Updates

Lead Judging Commences

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

finding-DeliveryPlace-settleAskTaker-closeBidTaker-wrong-makerinfo-token-address-addToken-balance

Valid high severity, In `settleAskTaker/closeBidTaker`, by assigning collateral token to user balance instead of point token, if collateral token is worth more than point, this can cause stealing of other users collateral tokens within the CapitalPool contract, If the opposite occurs, user loses funds based on the points they are supposed to receive

Appeal created

0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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