Summary
Because the createOffer function creates offerAddr and stockAddr using the current offerId but sets offerInfoMap[offerAddr]'s and stockInfoMap[stockAddr]'s id to the incremented offerId, the offerId that such offer's and stock's addresses correspond to and the offerId that such offer's and stock's id are set to are not consistent. Moreover, the id of the stocks created by the createOffer and createTaker function calls can be the same, and the id of the offers created by the createOffer and listOffer function calls can be the same. As a result, users can match incorrect offers or stocks when only the id of the offers or stocks that they want to match are known.
Vulnerability Details
At the beginning, offerId equals 0.
In a sequential order, the following createOffer function creates offerAddr and stockAddr using the current offerId, increments offerId by 1, and sets offerInfoMap[offerAddr]'s and stockInfoMap[stockAddr]'s id to the incremented offerId. For example, the created offer's offerAddr and stock's stockAddr can correspond to the offerId that equals 0 but their id are the offerId that equals 1.
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/PreMarkets.sol#L39-L157
function createOffer(CreateOfferParams calldata params) external payable {
...
if (params.points == 0x0 || params.amount == 0x0) {
revert Errors.AmountIsZero();
}
if (params.eachTradeTax > Constants.EACH_TRADE_TAX_DECIMAL_SCALER) {
revert InvalidEachTradeTaxRate();
}
if (params.collateralRate < Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
revert InvalidCollateralRate();
}
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(params.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
@> address offerAddr = GenerateAddress.generateOfferAddress(offerId);
@> address stockAddr = GenerateAddress.generateStockAddress(offerId);
if (makerInfoMap[makerAddr].authority != address(0x0)) {
revert MakerAlreadyExist();
}
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
@> offerId = offerId + 1;
{
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
);
}
makerInfoMap[makerAddr] = MakerInfo({
offerSettleType: params.offerSettleType,
authority: _msgSender(),
marketPlace: params.marketPlace,
tokenAddress: params.tokenAddress,
originOffer: offerAddr,
platformFee: 0,
eachTradeTax: params.eachTradeTax
});
@> 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
});
@> stockInfoMap[stockAddr] = StockInfo({
@> id: offerId,
stockStatus: StockStatus.Initialized,
stockType: params.offerType == OfferType.Ask
? StockType.Bid
: StockType.Ask,
authority: _msgSender(),
maker: makerAddr,
preOffer: address(0x0),
offer: offerAddr,
points: params.points,
amount: params.amount
});
...
}
The following createTaker function can be called to create a new stock for the offer created by the createOffer function call. This new stock's stockAddr corresponds to and its id is set to the current offerId. Then, this function increments offerId by 1. For example, for the stock created by the createTaker function call, the stockAddr can correspond to and id can be the offerId that equals 1. As a comparison, the stocks created by the createTaker and createOffer function calls can end up with the same id, such as 1.
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/PreMarkets.sol#L164-L284
function createTaker(address _offer, uint256 _points) external payable {
...
if (_points == 0x0) {
revert Errors.AmountIsZero();
}
OfferInfo storage offerInfo = offerInfoMap[_offer];
MakerInfo storage makerInfo = makerInfoMap[offerInfo.maker];
if (offerInfo.offerStatus != OfferStatus.Virgin) {
revert InvalidOfferStatus();
}
if (offerInfo.points < _points + offerInfo.usedPoints) {
revert NotEnoughPoints(
offerInfo.points,
offerInfo.usedPoints,
_points
);
}
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
{
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
}
ReferralInfo memory referralInfo = systemConfig.getReferralInfo(
_msgSender()
);
uint256 platformFeeRate = systemConfig.getPlatformFeeRate(_msgSender());
@> address stockAddr = GenerateAddress.generateStockAddress(offerId);
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
uint256 depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);
uint256 platformFee = depositAmount.mulDiv(
platformFeeRate,
Constants.PLATFORM_FEE_DECIMAL_SCALER
);
uint256 tradeTax = depositAmount.mulDiv(
makerInfo.eachTradeTax,
Constants.EACH_TRADE_TAX_DECIMAL_SCALER
);
ITokenManager tokenManager = tadleFactory.getTokenManager();
_depositTokenWhenCreateTaker(
platformFee,
depositAmount,
tradeTax,
makerInfo,
offerInfo,
tokenManager
);
offerInfo.usedPoints = offerInfo.usedPoints + _points;
@> stockInfoMap[stockAddr] = StockInfo({
@> id: offerId,
stockStatus: StockStatus.Initialized,
stockType: offerInfo.offerType == OfferType.Ask
? StockType.Bid
: StockType.Ask,
authority: _msgSender(),
maker: offerInfo.maker,
preOffer: _offer,
points: _points,
amount: depositAmount,
offer: address(0x0)
});
@> offerId = offerId + 1;
uint256 remainingPlatformFee = _updateReferralBonus(
platformFee,
depositAmount,
stockAddr,
makerInfo,
referralInfo,
tokenManager
);
makerInfo.platformFee = makerInfo.platformFee + remainingPlatformFee;
_updateTokenBalanceWhenCreateTaker(
_offer,
tradeTax,
depositAmount,
offerInfo,
makerInfo,
tokenManager
);
...
}
Afterwards, the following listOffer function can be called to create a new offer for the stock created by the createTaker function call. This new offer's offerAddr corresponds to and its id is set to the id of the associated stock. For example, for the offer created by the listOffer function call, the offerAddr can correspond to and id can be the offerId that equals 1, which is the id of the stock created by the createTaker function call. As a comparison, the offers created by the listOffer and createOffer function calls can end up with the same id, such as 1.
https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/PreMarkets.sol#L295-L396
function listOffer(
address _stock,
uint256 _amount,
uint256 _collateralRate
) external payable {
if (_amount == 0x0) {
revert Errors.AmountIsZero();
}
if (_collateralRate < Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
revert InvalidCollateralRate();
}
StockInfo storage stockInfo = stockInfoMap[_stock];
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
OfferInfo storage offerInfo = offerInfoMap[stockInfo.preOffer];
MakerInfo storage makerInfo = makerInfoMap[offerInfo.maker];
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
if (stockInfo.offer != address(0x0)) {
revert OfferAlreadyExist();
}
if (stockInfo.stockType != StockType.Bid) {
revert InvalidStockType(StockType.Bid, stockInfo.stockType);
}
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
_amount,
true,
Math.Rounding.Ceil
);
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
);
}
@> address offerAddr = GenerateAddress.generateOfferAddress(stockInfo.id);
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}
@> offerInfoMap[offerAddr] = OfferInfo({
@> id: stockInfo.id,
authority: _msgSender(),
maker: offerInfo.maker,
offerStatus: OfferStatus.Virgin,
offerType: offerInfo.offerType,
abortOfferStatus: AbortOfferStatus.Initialized,
points: stockInfo.points,
amount: _amount,
collateralRate: _collateralRate,
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});
stockInfo.offer = offerAddr;
...
}
Impact
For the offer and stock created by the createOffer function call, the offerId that such offer's offerAddr and stock's stockAddr correspond to and the offerId that such offer's and stock's id are set to are inconsistent. Users, who want to match such offer or stock, might apply the GenerateAddress.generateOfferAddress or GenerateAddress.generateStockAddress function' logics with such offer's or stock's id as the input but the resulting offer or stock address would incorrectly be for the offer or stock that is not the wanted offer or stock, which cause them to match incorrect offer or stock.
Since the id of the stocks created by the createOffer and createTaker functions can be the same, users, who apply the GenerateAddress.generateStockAddress function' logics with such id as the input, can only retrieve the address of the stock created by the createTaker function. These users are not able to match the stock created by the createOffer function if only knowing such stock's id.
Similarly, since the id of the offers created by the createOffer and listOffer functions can be the same, users, who apply the GenerateAddress.generateOfferAddress function' logics with such id as the input, can only retrieve the address of the offer created by the listOffer function. These users are not able to match the offer created by the createOffer function if only knowing such offer's id.
Tools Used
Manual Review
Recommended Mitigation
The createOffer function can be updated to only increment offerId after setting offerInfoMap[offerAddr]'s and stockInfoMap[stockAddr]'s id to the current offerId.
