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

Wrong balance update of takers after the settlement

fyamf

Summary

After the settlement, the taker instead of receiving marketPlaceInfo.tokenAddress, they receive makerInfo.tokenAddress. So, the balance of the Taker is updated incorrectly.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L198

Vulnerability Details

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
makerInfo.tokenAddress,
pointTokenAmount
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L198

  • This means that Bob can now withdraw 1100 USDC instead of 1100 marketPlaceInfo.tokenAddress. This is implemented incorrectly. If 1100 USDC worth more than 1100 marketPlaceInfo.tokenAddress, Bob gained, otherwise he loses.

PoC

In the following test, it is implementing the exact scenario above .

function test_ask_offer_turbo_usdc_PoC1() public {
address Alice = vm.addr(10); // maker
address Bob = vm.addr(11); // taker
​
////// Alice(maker) creates a ask offer
vm.startPrank(Alice);
deal(address(mockUSDCToken), Alice, 1.2 * 10 ** 18);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
vm.startPrank(Alice);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
vm.stopPrank();
////////////////
​
///// Bob(taker) creates a bid order
vm.startPrank(Bob);
deal(address(mockUSDCToken), Bob, type(uint256).max);
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
​
vm.stopPrank();
/////////////
​
///// owner updates the market and we have TGE
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
1.1 * 1e18,
block.timestamp - 1,
3600
);
vm.stopPrank();
//////////////
​
////// Alice settles
vm.startPrank(Alice);
deal(address(mockPointToken), Alice, 1100 * 10 ** 18);
​
mockPointToken.approve(address(tokenManager), 1100 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 1000);
​
vm.stopPrank();
/////////////
​
//////// Bob closes
vm.startPrank(Bob);
address stock1Addr = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stock1Addr);
vm.stopPrank();
/////////////
​
console.log(
"Bob balance of USDC in the protocol: ",
tokenManager.userTokenBalanceMap(
address(Bob),
address(mockUSDCToken),
TokenBalanceType.PointToken
)
);
​
console.log(
"Bob balance of mockPointToken in the protocol: ",
tokenManager.userTokenBalanceMap(
address(Bob),
address(mockPointToken),
TokenBalanceType.PointToken
)
);
}

The output is:

Bob balance of USDC in the protocol: 1100000000000000000000
Bob balance of mockPointToken in the protocol: 0

Impact

  • Wrong balance update of takers after the settlement

Tools Used

Recommendations

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
marketPlaceInfo.tokenAddress, // Here it should be modified
pointTokenAmount
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L198

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

Appeal created

fyamf Submitter
about 1 year ago
0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge 12 months 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.