Summary
If the creator of an Offer decides to terminate the transaction, the Taker will be unfairly forced to suffer financial losses.
Vulnerability Details
The creator of an Ask.offer can call the PreMarktes::abortAskOffer() function to abort the transaction and reclaim all their collateral.
function abortAskOffer(address _stock, address _offer) external {
uint256 makerRefundAmount;
if (transferAmount > totalDepositAmount) {
@> makerRefundAmount = transferAmount - totalDepositAmount;
} else {
@> makerRefundAmount = 0;
}
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
@> makerRefundAmount
);
offerInfo.abortOfferStatus = AbortOfferStatus.Aborted;
offerInfo.offerStatus = OfferStatus.Settled;
emit AbortAskOffer(_offer, _msgSender());
}
However, the creator of Bid.Taker has paid platformFee and tradeTax fees when calling PreMarktes::createTaker(), and the relevant amount of tradeTax is directly updated to the creator address of Ask.offer in the call of PreMarktes::_updateTokenBalanceWhenCreateTaker(). Even if PreMarktes::abortBidTaker() is called, the relevant fees of platformFee and tradeTax will not be refunded. It will inevitably lead to losses for the creator of Bid.Taker.
function createTaker(address _offer, uint256 _points) external payable {
uint256 depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);
@> uint256 platformFee = depositAmount.mulDiv(
platformFeeRate,
Constants.PLATFORM_FEE_DECIMAL_SCALER
);
@> uint256 tradeTax = depositAmount.mulDiv(
makerInfo.eachTradeTax,
Constants.EACH_TRADE_TAX_DECIMAL_SCALER
);
@> _updateTokenBalanceWhenCreateTaker(
_offer,
tradeTax,
depositAmount,
offerInfo,
makerInfo,
tokenManager
);
}
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
);
}
}
function abortBidTaker(address _stock, address _offer) external {
StockInfo storage stockInfo = stockInfoMap[_stock];
OfferInfo storage preOfferInfo = offerInfoMap[_offer];
if (stockInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}
if (stockInfo.preOffer != _offer) {
revert InvalidOfferAccount(stockInfo.preOffer, _offer);
}
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus(
StockStatus.Initialized,
stockInfo.stockStatus
);
}
if (preOfferInfo.abortOfferStatus != AbortOfferStatus.Aborted) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Aborted,
preOfferInfo.abortOfferStatus
);
}
uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points,
preOfferInfo.amount,
Math.Rounding.Floor
);
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
MakerInfo storage makerInfo = makerInfoMap[preOfferInfo.maker];
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
transferAmount
);
stockInfo.stockStatus = StockStatus.Finished;
emit AbortBidTaker(_offer, _msgSender());
}
Poc
To demonstrate the issue, add the following test code to test/PreMarkets.t.sol and run it:
function test_if_abortAskOffer() public {
uint256 userStart_mockUSDCToken_balance = mockUSDCToken.balanceOf(user);
uint256 user2Start_mockUSDCToken_balance = mockUSDCToken.balanceOf(user2);
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
vm.stopPrank();
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
vm.prank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
vm.startPrank(user2);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.abortBidTaker(stock1Addr, offerAddr);
vm.stopPrank();
console2.log("--------------- user balance ---------------");
balanceHelper(user);
console2.log("--------------- user2 balance ---------------");
balanceHelper(user2);
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(user);
tokenManager.withdraw(address(mockUSDCToken),TokenBalanceType.TaxIncome);
tokenManager.withdraw(address(mockUSDCToken),TokenBalanceType.SalesRevenue);
tokenManager.withdraw(address(mockUSDCToken),TokenBalanceType.MakerRefund);
vm.stopPrank();
vm.prank(user2);
tokenManager.withdraw(address(mockUSDCToken),TokenBalanceType.MakerRefund);
uint256 userEnd_mockUSDCToken_balance = mockUSDCToken.balanceOf(user);
uint256 user2End_mockUSDCToken_balance = mockUSDCToken.balanceOf(user2);
console2.log("The user's profit is:", userEnd_mockUSDCToken_balance - userStart_mockUSDCToken_balance);
console2.log("The user2's profit is:",int256(user2End_mockUSDCToken_balance) - int256(user2Start_mockUSDCToken_balance));
}
function balanceHelper(address _user) view public {
uint256 usermockUSDCTokenAmount_TaxIncome = tokenManager.userTokenBalanceMap(
address(_user),
address(mockUSDCToken),
TokenBalanceType.TaxIncome
);
console2.log("usermockUSDCTokenAmount_TaxIncome:",usermockUSDCTokenAmount_TaxIncome);
uint256 usermockUSDCTokenAmount_ReferralBonus = tokenManager.userTokenBalanceMap(
address(_user),
address(mockUSDCToken),
TokenBalanceType.ReferralBonus
);
console2.log("usermockUSDCTokenAmount_ReferralBonus:",usermockUSDCTokenAmount_ReferralBonus);
uint256 usermockUSDCTokenAmount_SalesRevenue = tokenManager.userTokenBalanceMap(
address(_user),
address(mockUSDCToken),
TokenBalanceType.SalesRevenue
);
console2.log("usermockUSDCTokenAmount_SalesRevenue:",usermockUSDCTokenAmount_SalesRevenue);
uint256 usermockUSDCTokenAmount_RemainingCash = tokenManager.userTokenBalanceMap(
address(_user),
address(mockUSDCToken),
TokenBalanceType.RemainingCash
);
console2.log("usermockUSDCTokenAmount_RemainingCash:",usermockUSDCTokenAmount_RemainingCash);
uint256 usermockUSDCTokenAmount_MakerRefund = tokenManager.userTokenBalanceMap(
address(_user),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log("usermockUSDCTokenAmount_MakerRefund:",usermockUSDCTokenAmount_MakerRefund);
uint256 usermockUSDCTokenAmount_PointToken = tokenManager.userTokenBalanceMap(
address(_user),
address(mockUSDCToken),
TokenBalanceType.PointToken
);
console2.log("usermockUSDCTokenAmount_PointToken:",usermockUSDCTokenAmount_PointToken);
}
The output from the PoC reveals the following key points:
-
The creator of Ask.offer(user) can call PreMarktes::abortAskOffer() at any time to terminate the transaction without any penalties. In fact, they may even profit due to the tradeTax.
-
The creator of Bid.Taker(user2) is forced to call PreMarktes::abortBidTaker() to retrieve the remaining collateral, which incurs a loss equal to platformFee + tradeTax.
-
A malicious Ask.offer creator can use MEV (Maximal Extractable Value) tactics to execute PreMarktes::abortAskOffer() before the administrator calls SystemConfig::updateMarket(), maximizing their tradeTax benefits.
-
The Bid.Taker is completely passive in this transaction, and despite not initiating the termination, they end up bearing the cost of the transaction's termination.
[PASS] test_if_abortAskOffer() (gas: 1064023)
Logs:
@> platformFee in createTaker: 50000000000000
@> tradeTax in createTaker: 300000000000000
--------------- user balance ---------------
usermockUSDCTokenAmount_TaxIncome: 300000000000000
usermockUSDCTokenAmount_ReferralBonus: 0
usermockUSDCTokenAmount_SalesRevenue: 10000000000000000
usermockUSDCTokenAmount_RemainingCash: 0
usermockUSDCTokenAmount_MakerRefund: 2000000000000000
usermockUSDCTokenAmount_PointToken: 0
--------------- user2 balance ---------------
usermockUSDCTokenAmount_TaxIncome: 0
usermockUSDCTokenAmount_ReferralBonus: 0
usermockUSDCTokenAmount_SalesRevenue: 0
usermockUSDCTokenAmount_RemainingCash: 0
usermockUSDCTokenAmount_MakerRefund: 10000000000000000
usermockUSDCTokenAmount_PointToken: 0
@> The user's profit is: 300000000000000 # tradeTax
@> The user2's profit is: -350000000000000 # platformFee + tradeTax
Code Snippet
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L164-L284
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L536-L635
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L645-L697
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L906-L949
Impact
The offer creator holds a significant advantage in the entire transaction. Even if they are unable to preempt the transaction before the administrator calls SystemConfig::updateMarket(), the collateral rate (CreateOfferParams:collateralRate) is set by the creator. Given that the value in the current state needs to be slightly above 10000 (e.g., 10001), the creator is unlikely to suffer any losses. (Refer to the related issue: If the offer creator refuses to execute DeliveryPlace::settleAskMaker(), the Taker will lose all the tokens they paid.)
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
@> 12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
In contrast, the Taker bears all the losses caused by any issues with the offer.
Therefore, before engaging in a transaction, the Taker must assess whether the offer is reliable, whether the transaction will proceed smoothly, and whether they might suffer any losses as a result. This situation creates an odd and unfair burden on the Taker.
Ultimately, if Takers frequently suffer losses due to failed transactions, it will erode trust in the Tadle platform. More critically, if the Takers disappear, so too will Tadle.
Tools Used
Manual Review
Recommendations
To address this issue, modify the code such that when the creator of an Ask.offer wishes to terminate the transaction by calling PreMarktes::abortAskOffer(), the platform fee and trade tax paid by all Takers should be deducted from the Ask.offer creator’s collateral based on the order status. Additionally, when PreMarktes::abortBidTaker() is executed, the full amount paid by the Taker should be refunded, as the transaction termination is not their fault.
