Summary
The DeliveryPlace::closeBidTaker() function incorrectly uses the makerInfo.tokenAddress to update the balance of the TokenBalanceType.PointToken branch in the user's userTokenBalanceMap mapping, leading to significant errors in the balance data within the mapping.
Vulnerability Details
The DeliveryPlace::closeBidTaker() function is designed to update the token balance and the StockStatus status after a transaction is completed by the createTaker() creator. Upon reviewing the code, it is evident that the same makerInfo.tokenAddress is used as a parameter twice when updating the userTokenBalanceMap mapping.
function closeBidTaker(address _stock) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
ITokenManager tokenManager = tadleFactory.getTokenManager();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
(
OfferInfo memory preOfferInfo,
@> MakerInfo memory makerInfo,
,
) = getOfferInfo(stockInfo.preOffer);
uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
@> makerInfo.tokenAddress,
userCollateralFee
);
uint256 pointTokenAmount = offerInfo.settledPointTokenAmount.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
@> makerInfo.tokenAddress,
pointTokenAmount
);
}
The makerInfo.tokenAddress corresponds to the token used for payment during the createTaker() call, which is correct for the TokenBalanceType.RemainingCash item. However, this address is incorrectly used for the TokenBalanceType.PointToken item, leading to significant errors. The provided PoC below demonstrates this issue in detail.
Poc
Add the following test code to test/PreMarkets.t.sol and execute it:
function testDeliveryPlace_closeBidTaker_error() public {
console2.log("mockPointToken address:",address(mockPointToken));
console2.log("mockUSDCToken address:",address(mockUSDCToken));
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
address userOfferAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(userOfferAddr, 1000);
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(userOfferAddr, 1000);
vm.stopPrank();
assertEq(mockPointToken.balanceOf(address(capitalPool)),1e19);
address user2StockAddr = GenerateAddress.generateStockAddress(1);
vm.prank(user2);
deliveryPlace.closeBidTaker(user2StockAddr);
uint256 user2mockPointTokenAmount_PointToken = tokenManager.userTokenBalanceMap(
address(user2),
address(mockPointToken),
TokenBalanceType.PointToken
);
console2.log("user2mockPointTokenAmount-PointToken:",user2mockPointTokenAmount_PointToken);
assertEq(user2mockPointTokenAmount_PointToken,0);
uint256 user2mockUSDCTokenAmount_PointToken = tokenManager.userTokenBalanceMap(
address(user2),
address(mockUSDCToken),
TokenBalanceType.PointToken
);
console2.log("user2mockUSDCTokenAmount-PointToken:",user2mockUSDCTokenAmount_PointToken);
assertEq(user2mockUSDCTokenAmount_PointToken,1e19);
uint256 capitalPool_mockUSDCTokenAmount = mockUSDCToken.balanceOf(address(capitalPool));
console2.log("capitalPool USDC token balance:", capitalPool_mockUSDCTokenAmount);
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(user2);
vm.expectRevert();
tokenManager.withdraw(address(mockUSDCToken),TokenBalanceType.PointToken);
vm.stopPrank();
}
Code Snippet
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L89-L212
Impact
The incorrect usage of makerInfo.tokenAddress in the DeliveryPlace::closeBidTaker() function results in serious errors in the user's balance data in the userTokenBalanceMap mapping.
Tools Used
Manual Review
Recommendations
Call getOfferInfo() to retrieve MarketPlaceInfo memory marketPlaceInfo.
Use the marketPlaceInfo.tokenAddress parameter when updating the TokenBalanceType.PointToken branch balance:
function closeBidTaker(address _stock) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
ITokenManager tokenManager = tadleFactory.getTokenManager();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
// SNIP...
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
+ MarketPlaceInfo memory marketPlaceInfo
,
) = getOfferInfo(stockInfo.preOffer);
// SNIP...
uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
userCollateralFee
);
uint256 pointTokenAmount = offerInfo.settledPointTokenAmount.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
- makerInfo.tokenAddress,
+ marketPlaceInfo.tokenAddress,
pointTokenAmount
);
// SNIP...
}
