Summary
The createOffer function in the PreMarkets contract contains a logic flaw that can lead to the generation of duplicate IDs for StockInfo structs. This issue arises from the timing of when the offerId is incremented.
Vulnerability Details
The offerId state variable is incremented before initializing the OfferInfo and StockInfo structs in the createOffer function. This causes the structs to be initialized with the new, incremented offerId.
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;
offerInfoMap[offerAddr] = OfferInfo({
@>> id: offerId,
authority: _msgSender(),
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
});
Consequently, when createTaker function is called, it also generates a stockAddr using the same offerId, which has already been incremented. This creates a potential collision where two StockInfo structs might have the same ID.
@>> address stockAddr = GenerateAddress.generateStockAddress(offerId);
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
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;
Impact
This discrepancy leads to a situation where the StockInfo struct created in createTaker can end up with an id that is already in use by a StockInfo created in a previous createOffer call. Consequently, the stockInfoMap can end up with entries that have duplicate id values.
Tools Used
Manual-Review
Recommendations
To fix this issue, the offerId should be incremented after the MakerInfo, OfferInfo, and StockInfo structs have been fully initialized in createOffer function .
/// @dev generate address for maker, offer, stock.
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;
{
/// @dev transfer collateral from _msgSender() to capital pool
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
);
}
/// @dev update maker info
makerInfoMap[makerAddr] = MakerInfo({
offerSettleType: params.offerSettleType,
authority: _msgSender(),
marketPlace: params.marketPlace,
tokenAddress: params.tokenAddress,
originOffer: offerAddr,
platformFee: 0,
eachTradeTax: params.eachTradeTax
});
/// @dev update offer info
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
});
/// @dev update stock info
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
});
+ offerId = offerId + 1;
