Summary
Calling the settleAskTaker function can increase the corresponding offerInfo.authority's collateral token balance for TokenBalanceType.PointToken by settledPointTokenAmount though such offerInfo.authority's point token balance for TokenBalanceType.PointToken should be increased by settledPointTokenAmount instead. Thus, such offerInfo.authority would not be able to claim such settledPointTokenAmount of the point token that is entitled to him.
Similarly, calling the closeBidTaker function can increase the corresponding _msgSender()'s collateral token balance for TokenBalanceType.PointToken by pointTokenAmount though such _msgSender()'s point token balance for TokenBalanceType.PointToken should be increased by pointTokenAmount instead. Therefore, such _msgSender() would not be able to claim such pointTokenAmount of the point token that is entitled to him.
Vulnerability Details
The settleAskTaker function below executes tokenManager.addTokenBalance(TokenBalanceType.PointToken, offerInfo.authority, makerInfo.tokenAddress, settledPointTokenAmount). Here, makerInfo.tokenAddress is the collateral token address, which is the CreateOfferParams struct's tokenAddress, and is not the point token address, which should be the MarketPlaceInfo struct's tokenAddress. Hence, such tokenManager.addTokenBalance function call increases the offerInfo.authority's collateral token balance for TokenBalanceType.PointToken by settledPointTokenAmount in the token manager; however, the offerInfo.authority's point token balance for TokenBalanceType.PointToken should be increased by settledPointTokenAmount instead.
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/interfaces/IPerMarkets.sol#L284-L306
...
* @param tokenAddress the collateral token address of offer.
...
*/
struct CreateOfferParams {
address marketPlace;
@> address tokenAddress;
uint256 points;
uint256 amount;
uint256 collateralRate;
uint256 eachTradeTax;
OfferType offerType;
OfferSettleType offerSettleType;
}
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/interfaces/ISystemConfig.sol#L128-L146
...
* @param tokenAddress the point token address
...
*/
struct MarketPlaceInfo {
bool fixedratio;
MarketPlaceStatus status;
@> address tokenAddress;
uint256 tokenPerPoint;
uint256 tge;
uint256 settlementPeriod;
}
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/DeliveryPlace.sol#L335-L433
function settleAskTaker(address _stock, uint256 _settledPoints) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
MarketPlaceStatus status
) = getOfferInfo(stockInfo.preOffer);
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus();
}
if (marketPlaceInfo.fixedratio) {
revert FixedRatioUnsupported();
}
if (stockInfo.stockType == StockType.Bid) {
revert InvalidStockType();
}
if (_settledPoints > stockInfo.points) {
revert InvalidPoints();
}
if (status == MarketPlaceStatus.AskSettling) {
if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint *
_settledPoints;
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
tokenManager.addTokenBalance(
@> TokenBalanceType.PointToken,
@> offerInfo.authority,
@> makerInfo.tokenAddress,
@> settledPointTokenAmount
);
}
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
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
offerInfo.authority,
makerInfo.tokenAddress,
collateralFee
);
}
perMarkets.settleAskTaker(
stockInfo.preOffer,
_stock,
_settledPoints,
settledPointTokenAmount
);
...
}
Similarly, the following closeBidTaker function executes tokenManager.addTokenBalance(TokenBalanceType.PointToken, _msgSender(), makerInfo.tokenAddress, pointTokenAmount), where makerInfo.tokenAddress is the collateral token address, which is the CreateOfferParams struct's tokenAddress, and is not the point token address, which should be the MarketPlaceInfo struct's tokenAddress. In this case, such tokenManager.addTokenBalance function call increases the _msgSender()'s collateral token balance for TokenBalanceType.PointToken by pointTokenAmount in the token manager; yet, the _msgSender()'s point token balance for TokenBalanceType.PointToken should be increased by pointTokenAmount instead.
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/DeliveryPlace.sol#L96-L212
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);
...
}
Impact
The offerInfo.authority corresponding to the settleAskTaker function call would not be able to claim the corresponding settledPointTokenAmount of the point token that is entitled to him. Similarly, the _msgSender() corresponding to the closeBidTaker function call would not be able to claim the corresponding pointTokenAmount of the point token that is entitled to him.
Tools Used
Manual Review
Recommended Mitigation
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/DeliveryPlace.sol#L384-L389 can be updated to the following code.
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
marketPlaceInfo.tokenAddress,
settledPointTokenAmount
);
Moreover, the closeBidTaker function can be updated to use the corresponding MarketPlaceInfo struct's tokenAddress instead of makerInfo.tokenAddress for calling the tokenManager.addTokenBalance function for TokenBalanceType.PointToken.
