Tadle

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

Wrong transfer token address when settle ask taker

Summary

When settle ask taker, TokenManager transfer point token from wrong address , and add token balance with wrong point address.

Vulnerability Details

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

In DeliveryPlace, there is a settleAskTaker function.

function settleAskTaker(address _stock, uint256 _settledPoints) external {
...
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
@>> _msgSender(), // @audit issue 1
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
@>> makerInfo.tokenAddress, // @audit issue 2
settledPointTokenAmount
);
}
...
}

It expects:

  • tokenManager.tillIn will transfer the point token from Taker ( who called createTaker function) to capitalPoolAddr

  • tokenManager.addTokenBalance will add token amount to Offer

But In the code, it implements wrong.

  • Issue 1: call tillIn function with wrong address, _msgSender() is not Taker

  • Issue 2: We need to add Point token balance to the Offer, but it is not Point token address.

The below is POC. It will fail.

function test_settleAskTaker_point_balance() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
vm.stopPrank();
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);
uint256 user2PTBalance00 = mockPointToken.balanceOf(user2);
deliveryPlace.settleAskTaker(stock1Addr, 499);
uint256 user2PTBalance01 = mockPointToken.balanceOf(user2);
uint256 pointMoveAmount = 0.01 * 1e18 * 499;
assertEq(pointMoveAmount, user2PTBalance00 - user2PTBalance01);
uint256 userPointTokenBalance = tokenManager.userTokenBalanceMap(
address(user),
address(mockPointToken),
TokenBalanceType.PointToken
);
assertEq(pointMoveAmount, userPointTokenBalance);
console2.log("userPointTokenBalance", userPointTokenBalance);
tokenManager.withdraw(address(mockPointToken), TokenBalanceType.PointToken);
vm.stopPrank();
}
[FAIL. Reason: assertion failed] test_settleAskTaker_point_balance() (gas: 1160345)
Logs:
Error: a == b not satisfied [uint]
Expected: 4990000000000000000
Actual: 0
Error: a == b not satisfied [uint]
Expected: 4990000000000000000
Actual: 0
userPointTokenBalance 0

Impact

Wrong transfer token address and token balance when settle ask taker

Tools Used

Manual Review

Recommendations

We could fix like the below

function settleAskTaker(address _stock, uint256 _settledPoints) external {
...
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
- // _msgSender(), // @audit
+ stockInfo.authority, // @audit
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
- // makerInfo.tokenAddress, // @audit
+ marketPlaceInfo.tokenAddress, // @audit
settledPointTokenAmount
);
}
...
}
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.