Summary
Inconsistency between the id stored in StockInfo and the id used to generate the StockInfo address.
Vulnerability Details
When the user creates an offer using the function createOffer(CreateOfferParams calldata params), MakerInfo and OfferInfo are created with the last id + 1. These two elements should, in principle, be created with the last id. As shown in the code, the id is incremented beforehand.
function createOffer(CreateOfferParams calldata params) external payable {
.......
@> 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
});
emit CreateOffer(
offerAddr, makerAddr, stockAddr, params.marketPlace, _msgSender(), params.points, params.amount
);
}
Impact
There is therefore an inconsistency between the id used to generate the addresses offerAddr and stockAddr and the one contained in OfferInfo and StockInfo.
For example:
There is therefore an inconsistency between the id used to generate the addresses offerAddr and stockAddr and the one contained in OfferInfo and StockInfo.
For example:
1 - user1 creates an offer with createOffer, the id of the stock will be 1, and the id used to generate the stock address is 0.
2- user1 creates a taker offer with createTaker and then relists their offer with listOffer, the id of the stock will still be 1, and the id used to generate the stock address is 1.
3 - user1 creates a new offer with createOffer, the id of the stock will be 3, and the id used to generate the stock address is 2.
There has never been a stock with id 2, so there is no longer any consistency between the values.
Tools Used
Manual Review
Recommendations
Il faudrait creer OfferInfo et StockInfo et ensuite les incréementer
function createOffer(CreateOfferParams calldata params) external payable {
.......
/// @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, //@audit tradeTax is not used
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;
emit CreateOffer(
offerAddr, makerAddr, stockAddr, params.marketPlace, _msgSender(), params.points, params.amount
);
}
