Summary
Offers can NOT be fully filled due to wrong validation
Vulnerability Details
The protocol offers users the ability to list funds in a bid-ask order book like market. When a market maker list an offer they have to provide collateral and also amount of points that the offer will hold.
function createOffer(CreateOfferParams calldata params) external payable {
* @dev points and amount must be greater than 0
* @dev eachTradeTax must be less than 100%, decimal scaler is 10000
* @dev collateralRate must be more than 100%, decimal scaler is 10000
*/
if (params.points == 0x0 || params.amount == 0x0) {
revert Errors.AmountIsZero();
}
...
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
});
...
}
points are proportionally tied to the amount of tokens deposited. Take for example an Ask maker deposits 200 tokenA and sets 100 points (2:1). When a taker wants to buy 50 point that would equal 100 tokenA. Points are used to segregate the initial offer into smaller ones if needed and as a currency.
As seen in the createTaker function when an order is filled the usedPoints amount of the order is updated with the equivalent amount of points.
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];
...
=> offerInfo.usedPoints = offerInfo.usedPoints + _points;
...
The problem is that due to a wrong check in the createTaker function the order can NOT be fully filled - all points used - offerInfo.usedPoints == offerInfo.points
function createTaker(address _offer, uint256 _points) external payable {
...
=> if (offerInfo.points < _points + offerInfo.usedPoints) {
revert NotEnoughPoints(
offerInfo.points,
offerInfo.usedPoints,
_points
);
}
Impact
Loss of funds since the maker has to either close the offer to get all (or part) of his funds back or settle it in which case they lose some funds.
Tools Used
Manual Review, Protocol Docs
Recommendations
In the createTaker function ket users be able to fully fill maker offers
function createTaker(address _offer, uint256 _points) external payable {
/**
* @dev offer must be virgin
* @dev points must be greater than 0
* @dev total points must be greater than used points plus _points
*/
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) {
+ if (offerInfo.points <= _points + offerInfo.usedPoints) {
revert NotEnoughPoints(
offerInfo.points,
offerInfo.usedPoints,
_points
);
}
/// @dev market place must be online
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
{
...
