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

When user call closeBidTaker after settleAskMaker, TokenManager add wrong token address's balance for pointTokenAmount

danielwang8824

Summary

When user call closeBidTaker, TokenManager add wrong token address's balance for pointTokenAmount.

Vulnerability Details

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

When user call closeBidTaker after settleAskMaker, tokenManager will add Point token balance to the taker(caller).

function closeBidTaker(address _stock) external {
...
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
@>> makerInfo.tokenAddress,
pointTokenAmount
);
...
}

But It used wrong point token address. makerInfo.tokenAddress is collateral token address, not Point Token address.

The below is POC unit test code.

function test_closeBidTaker_point_token_balance() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
vm.startPrank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
address offer1Addr = GenerateAddress.generateOfferAddress(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);
uint256 userPTBalance00 = mockPointToken.balanceOf(user);
deliveryPlace.settleAskMaker(offerAddr, 199);
uint256 pointMoveAmount = 0.01 * 1e18 * 199;
uint256 userPTBalance01 = mockPointToken.balanceOf(user);
assertEq(userPTBalance00 - userPTBalance01, pointMoveAmount);
vm.stopPrank();
​
vm.startPrank(user2);
deliveryPlace.closeBidTaker(stock1Addr);
uint256 user2PTBalance = tokenManager.userTokenBalanceMap(
address(user2),
address(mockPointToken),
TokenBalanceType.PointToken
);
assertEq(user2PTBalance, pointMoveAmount);
vm.stopPrank();
}

It expect success but it got fail.

[FAIL. Reason: assertion failed] test_closeBidTaker_point_token_balance() (gas: 1177442)
Logs:
Error: a == b not satisfied [uint]
Expected: 1990000000000000000
Actual: 0

Impact

TokenManager add wrong token address's balance, so user could not withdraw point token, but they could withdraw collateral token of same amount. It will cause critical issue.

Tools Used

Manual Review

Recommendations

We have to use point token address when add token balance.

In DeliveryPlace.sol, we have to fix like the below.

function closeBidTaker(address _stock) external {
...
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
+ MarketPlaceInfo memory marketPlaceInfo, // @audit
​
) = getOfferInfo(stockInfo.preOffer);
...
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
- makerInfo.tokenAddress, // @audit
+ marketPlaceInfo.tokenAddress, // @audit
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.