Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Point sellers cannot settle `BID` offer owners, due to invalid `msg.sender` validation in `DeliveryPlace::settleAskTaker(...)`

0xb0k0

Summary

Tadle allows users to create BID offers to buy points from sellers. After the market settles, sellers need to settle the sold points to the buyers, otherwise they will lose their collateral. Currently, BID offer sellers are unable to settle their tokens due to invalid msg.sender validation.

Vulnerability Details

When a seller likes a BID offer, he/she creates an ASK stock to sell points, by depositing collateral. If he/she fails to do so the buyer will receive the corresponding collateral. Initially, the trades are only kept internally, where no points are traded until the marketplace reaches AskSettling state. When this happens, sellers need to invoke DeliveryPlace::settleAskTaker(...) on the ASK stock so that he/she can transfer the settled points to the DeliveryPlace, which then assigns the points to the buyer. However, DeliveryPlace::settleAskTaker(...) can only be called by the offer owner, which is the buyer, making it impossible for the seller to settle, thus making him lose his deposited collateral.

function settleAskTaker(address _stock, uint256 _settledPoints) external {
__SNIP__
if (status == MarketPlaceStatus.AskSettling) {
@> if (_msgSender() != offerInfo.authority) { // offerInfo.authority is the buyer, not the seller
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
__SNIP__
}

The below PoC shows how the exploit can occur. I have fixed the PreMarkets.sol contract name as it was PreMarktes. It can be run by adding the snippets in PreMarkets.t.sol and running forge test --mt testSellerCantSettleBidOffer -vv. I am using the following setup for the tests:

PoC Set-up
function setUp() public {
// deploy mocks
weth9 = new WETH9();
​
TadleFactory tadleFactory = new TadleFactory(user1);
​
mockUSDCToken = new MockERC20Token();
mockPointToken = new MockERC20Token();
​
SystemConfig systemConfigLogic = new SystemConfig();
CapitalPool capitalPoolLogic = new CapitalPool();
TokenManager tokenManagerLogic = new TokenManager();
PreMarkets preMarketsLogic = new PreMarkets();
DeliveryPlace deliveryPlaceLogic = new DeliveryPlace();
​
bytes memory deploy_data = abi.encodeWithSelector(INITIALIZE_OWNERSHIP_SELECTOR, user1);
vm.startPrank(user1);
​
address systemConfigProxy =
tadleFactory.deployUpgradeableProxy(1, address(systemConfigLogic), bytes(deploy_data));
​
address preMarketsProxy = tadleFactory.deployUpgradeableProxy(2, address(preMarketsLogic), bytes(deploy_data));
address deliveryPlaceProxy =
tadleFactory.deployUpgradeableProxy(3, address(deliveryPlaceLogic), bytes(deploy_data));
address capitalPoolProxy = tadleFactory.deployUpgradeableProxy(4, address(capitalPoolLogic), bytes(deploy_data));
address tokenManagerProxy =
tadleFactory.deployUpgradeableProxy(5, address(tokenManagerLogic), bytes(deploy_data));
​
vm.stopPrank();
// attach logic
systemConfig = SystemConfig(systemConfigProxy);
capitalPool = CapitalPool(capitalPoolProxy);
tokenManager = TokenManager(tokenManagerProxy);
preMarkets = PreMarkets(preMarketsProxy);
deliveryPlace = DeliveryPlace(deliveryPlaceProxy);
​
vm.startPrank(user1);
// initialize
systemConfig.initialize(basePlatformFeeRate, baseReferralRate);
tokenManager.initialize(address(weth9));
address[] memory tokenAddressList = new address[](2);
​
tokenAddressList[0] = address(mockUSDCToken);
tokenAddressList[1] = address(weth9);
​
tokenManager.updateTokenWhiteListed(tokenAddressList, true);
​
// create market place
systemConfig.createMarketPlace("Backpack", false);
systemConfig.updateMarket("Backpack", address(mockUSDCToken), 0.01 * 1e18, block.timestamp - 1, 3600);
vm.stopPrank();
​
deal(address(mockUSDCToken), user, 10000 * 10 ** 18);
deal(address(mockPointToken), user, 10000 * 10 ** 18);
deal(user, 100 * 10 ** 18);
​
deal(address(mockUSDCToken), user1, 10000 * 10 ** 18);
deal(address(mockUSDCToken), user2, 10000 * 10 ** 18);
deal(address(mockUSDCToken), user3, 10000 * 10 ** 18);
​
deal(address(mockPointToken), user2, 10000 * 10 ** 18);
deal(address(mockPointToken), user3, 10000 * 10 ** 18);
​
marketPlace = GenerateAddress.generateMarketPlaceAddress("Backpack");
​
vm.warp(1719826275);
​
vm.prank(user);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.prank(user);
mockPointToken.approve(address(tokenManager), type(uint256).max);
​
vm.startPrank(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
mockPointToken.approve(address(tokenManager), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(user3);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
mockPointToken.approve(address(tokenManager), type(uint256).max);
vm.stopPrank();
​
capitalPool.approve(address(mockUSDCToken)); // I explicitly set the approval here as it is not working properly in the code; this is shown in my other issue
}
PoC
function testSellerCantSettleBidOffer() public {
vm.prank(user);
preMarkets.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1e18, 10000, 300, OfferType.Bid, OfferSettleType.Turbo
)
); // user wants to buy 1000 points
​
address offerAddrUserTurbo = GenerateAddress.generateOfferAddress(0);
vm.prank(user2);
preMarkets.createTaker(offerAddrUserTurbo, 1000); // user2 sells the 1000 points from user
​
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 1e18, block.timestamp - 1, 3600); // Market is updated and one collateral token is worth 1e18 points
​
vm.prank(user1);
systemConfig.updateMarketPlaceStatus("Backpack", MarketPlaceStatus.AskSettling);
​
address stockAddrUserTurbo = GenerateAddress.generateStockAddress(1);
vm.startPrank(user2);
vm.expectRevert(Errors.Unauthorized.selector);
deliveryPlace.settleAskTaker(stockAddrUserTurbo, 1000); // User2 tries to settle the bid offer, but fails as only the maker can settle the bid offer
}

Impact

Sellers can't settle BID offers, leading to loss of collateral.

Tools Used

Manual review

Recommendations

Change the msg.sender validation in DeliveryPlace::settleAskTaker(...) to check for stockInfo.authority

@@ -358,7 +362,7 @@ contract DeliveryPlace is DeliveryPlaceStorage, Rescuable, IDeliveryPlace {
}
if (status == MarketPlaceStatus.AskSettling) {
- if (_msgSender() != offerInfo.authority) {
+ if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
} else {
Updates

Lead Judging Commences

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

finding-PreMarkets-settleAskTaker-wrong-stock-authority

Valid high severity, when taker offers are created pointing to a `offer`, the relevant `stockInfoMap` offers are created with the owner of the offer aka `authority`, set as the creater of the offer, as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L245). Because of the wrong check within settleAskTaker, it will permanently DoS the final settlement functionality for taker offers for the maker that listed the original offer, essentially bricking the whole functionality of the market i.e. maker will always get refunded the original collateral, and takers will never be able to transact the original points put up by the maker. This occurs regardless of market mode.

Support

FAQs

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