Tadle

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

Incorrect Token Address in settleAskTaker and closeBidTaker Functions

Summary

In the DeliveryPlace.sol contract of the Tadle protocol, the settleAskTaker and closeBidTaker functions incorrectly use makerInfo.tokenAddress when adding PointToken balances. This error can prevent buyers from receiving their point tokens from the CapitalPool, which is a critical flaw in the contract's logic.

Vulnerability Details

Within both the settleAskTaker and closeBidTaker functions, the contract adds PointToken balances using makerInfo.tokenAddress. However, the correct address to be used is marketPlaceInfo.tokenAddress. The incorrect address leads to a misallocation of tokens, disrupting the distribution of PointTokens to buyers.

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L195-L200

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L384-L389

Impact

This vulnerability can result in buyers not receiving their point tokens, causing significant financial loss and breaking the contract's intended functionality. The inability to distribute point tokens correctly could undermine user trust and disrupt the overall marketplace operations.

Proof of Concepts

This is the test code of the vulnerability in closeBidTaker.

function test_ask_offer_turbo_usdc() public {
vm.prank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
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(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 500);
vm.stopPrank();
vm.prank(user2);
deliveryPlace.closeBidTaker(stock1Addr);
console2.log("user usdc balance: ", mockUSDCToken.balanceOf(user));
console2.log("user2 usdc balance: ", mockUSDCToken.balanceOf(user2));
console2.log("user point balance: ", mockPointToken.balanceOf(user));
console2.log("user2 point balance: ", mockPointToken.balanceOf(user2));
console2.log("");
capitalPool.approve(address(mockUSDCToken));
capitalPool.approve(address(mockPointToken));
vm.startPrank(user);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.TaxIncome);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
vm.stopPrank();
vm.startPrank(user2);
tokenManager.withdraw(address(mockPointToken), TokenBalanceType.PointToken);
vm.expectRevert();
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.PointToken);
vm.stopPrank();
console2.log("user usdc balance: ", mockUSDCToken.balanceOf(user));
console2.log("user2 usdc balance: ", mockUSDCToken.balanceOf(user2));
console2.log("user point balance: ", mockPointToken.balanceOf(user));
console2.log("user2 point balance: ", mockPointToken.balanceOf(user2));
}

The result of test is like this.

user usdc balance: 99999999988000000000000000
user2 usdc balance: 99999999994825000000000000
user point balance: 99999995000000000000000000
user2 point balance: 100000000000000000000000000
user usdc balance: 100000000005150000000000000
user2 usdc balance: 99999999994825000000000000
user point balance: 99999995000000000000000000
user2 point balance: 100000000000000000000000000 // receive fail
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
│ └─ ← [Return]
├─ [0] VM::startPrank(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69)
│ └─ ← [Return]
├─ [3530] UpgradeableProxy::withdraw(MockERC20Token: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9], 5)
│ ├─ [3015] TokenManager::withdraw(MockERC20Token: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9], 5) [delegatecall]
│ │ └─ ← [Stop] // stopped
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [5233] UpgradeableProxy::withdraw(MockERC20Token: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 5)
│ ├─ [4714] TokenManager::withdraw(MockERC20Token: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 5) [delegatecall]
│ │ ├─ [534] TadleFactory::relatedContracts(4) [staticcall]
│ │ │ └─ ← [Return] UpgradeableProxy: [0x76006C4471fb6aDd17728e9c9c8B67d5AF06cDA0]
│ │ ├─ [1334] MockERC20Token::transferFrom(UpgradeableProxy: [0x76006C4471fb6aDd17728e9c9c8B67d5AF06cDA0], 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, 5000000000000000000 [5e18])
│ │ │ └─ ← [Revert] ERC20InsufficientBalance(0x76006C4471fb6aDd17728e9c9c8B67d5AF06cDA0, 25000000000000 [2.5e13], 5000000000000000000 [5e18])
│ │ └─ ← [Revert] TransferFailed() // reverted
│ └─ ← [Revert] TransferFailed()

As you can see, the bid taker can't get points from CapitalPool.

Tools Used

Manual code review

Recommendations

Replacing makerInfo.tokenAddress with marketPlaceInfo.tokenAddress in the settleAskTaker and closeBidTaker functions can fix it.

(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
) = getOfferInfo(stockInfo.preOffer);
----------------------------------------------------------------------
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
marketPlaceInfo.tokenAddress, // Correct address
pointTokenAmount
);
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

Support

FAQs

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