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

`PreMarkets:abortBidTaker` calculates the refund of an offer that has an active taker incorrectly, resulting in a loss of funds.

brutalclowney

Summary

PreMarkets:abortBidTaker is called after the user who created the offer aborts the transaction. This is in an attempt to recover the funds that they have spent on the transaction. The abortBidOffer function calculates the refund using the getDepositAmount function which calculates the deposit amount based on the offer points and the offer amount. This is not directly correlated with the takers deposited funds, resulting in a miscalculation of the deposit back to the taker.

Vulnerability Details

we can see the function calculate deposit here:

```solidity
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) {
return _amount;
}
​
/// @dev ask order
if (_offerType == OfferType.Ask && !_isMaker) {
return _amount;
}
​
return
Math.mulDiv(
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);
}

the _amount is calculated using the line in abortBidTaker :

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points, //Offer info points
preOfferInfo.amount, //Offer info amount
Math.Rounding.Floor
);

add the following test to the test suite:

​
function test_abort_turbo_offer() public {
//log refund balances before
uint256 userRefundBalanceBefore = tokenManager.userTokenBalanceMap(
address(user),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log("user refund balance before", userRefundBalanceBefore);
​
uint256 user1RefundBalanceBefore = tokenManager.userTokenBalanceMap(
address(user1),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log("user1 refund balance before", user1RefundBalanceBefore);
​
//log USDC balances before
uint256 userUSDTBalance0 = mockUSDCToken.balanceOf(user);
uint256 userUSDTBalance1 = mockUSDCToken.balanceOf(user1);
console2.log("user balance at start", userUSDTBalance0);
console2.log("user1 balance at start", userUSDTBalance1);
​
//log the point balances before
uint256 userPointBalanceBefore = mockPointToken.balanceOf(user);
console2.log("user overall point balance before", userPointBalanceBefore);
uint256 user1PointBalanceBefore = mockPointToken.balanceOf(user1);
console2.log("user1 overall point balance before", user1PointBalanceBefore);
​
//create an offer from the user
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
100 * 1e18,
10000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
//log the USDC balances after the offer
uint256 userUSDTBalance2 = mockUSDCToken.balanceOf(user);
uint256 userUSDTBalance3 = mockUSDCToken.balanceOf(user1);
console2.log("user balance after offer", userUSDTBalance2);
console2.log("user1 Balance after offer", userUSDTBalance3);
​
//check the overall point balance of users after the offer
uint256 userPointBalanceOffer = mockPointToken.balanceOf(user);
console2.log("user overall point balance after offer", userPointBalanceOffer);
uint256 user1PointBalanceOffer = mockPointToken.balanceOf(user1);
console2.log("user1 overall point balance after offer", user1PointBalanceOffer);
​
//create a taker from user1
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
preMarktes.createTaker(offerAddr, 500);
vm.stopPrank();
​
//log the USDC balances after the taker
uint256 userUSDTBalance4 = mockUSDCToken.balanceOf(user);
uint256 userUSDTBalance5 = mockUSDCToken.balanceOf(user1);
console2.log("user balance after taker", userUSDTBalance4);
console2.log("user1 Balance after taker", userUSDTBalance5);
​
//abort the offer from the user
vm.startPrank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
vm.stopPrank();
​
//log the stockPoints to demonstrate in deposit calculation
StockInfo memory stockInfo = preMarktes.getStockInfo(stockAddr);
uint256 stockPoints = stockInfo.points;
console2.log("stock points", stockPoints);
​
//offer is now aborted, so we now abort the taker from user1
vm.startPrank(user1);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.abortBidTaker(stock1Addr, offerAddr);
vm.stopPrank();
​
//log the USDC balances after the aborts
uint256 userUSDTBalance6 = mockUSDCToken.balanceOf(user);
uint256 userUSDTBalance7 = mockUSDCToken.balanceOf(user1);
console2.log("user balance after abort", userUSDTBalance6);
console2.log("user1 Balance after abort", userUSDTBalance7);
​
​
//log refund balances after the aborts
uint256 userRefundBalanceAfter = tokenManager.userTokenBalanceMap(
address(user),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log("user Refund balance After", userRefundBalanceAfter);
​
uint256 user1RefundBalanceAfter = tokenManager.userTokenBalanceMap(
address(user1),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log("user1 Refund Balance After", user1RefundBalanceAfter);
​
//transaction is now finished and settled yet user1 is credited no refund and has no points in the system.
​
//check the claimable balance of the users for the point token
uint256 userPointBalance = tokenManager.userTokenBalanceMap(
address(user),
address(mockPointToken),
TokenBalanceType.PointToken
);
console2.log("user claimable point balance", userPointBalance);
​
uint256 user1PointBalance = tokenManager.userTokenBalanceMap(
address(user),
address(mockPointToken),
TokenBalanceType.PointToken
);
console2.log("user1 claimable point balance", user1PointBalance);
​
//check the overall point balance of users
uint256 userPointBalanceAfter = mockPointToken.balanceOf(user);
console2.log("user overall point balance after", userPointBalanceAfter);
uint256 user1PointBalanceAfter = mockPointToken.balanceOf(user1);
console2.log("user1 overall point balance after", user1PointBalanceAfter);
}

in this example the offer is created with a user asking for 100USDC for 1000 points.

these values represent in the function as

  • stockInfo.points = 1000

  • preOfferInfopoints = 1000

  • amount = 1e20 (100USDC in wei)
    when we run that calculation in the depositAmount calculation we get this output:

deposit amount = 1000 x 1000 / 1e20
= 0.00000000000000000001
after flooring = 0

this deposit amount is then passed as 0 to the transferAmount which passes 0 to the getDepositAmount where the decimal scaler is located, however the 0 value for amount renders this implementation useless. leaving the taker with funds locked in the contract as they have no claimable allocations or points earned from the transaction.

this is displayed in the logs from the test

user refund balance before 0
user1 refund balance before 0
user balance at start 100000000000000000000000000
user1 balance at start 100000000000000000000000000
user overall point balance before 100000000000000000000000000
user1 overall point balance before 0
user balance after offer 99999900000000000000000000
user1 Balance after offer 100000000000000000000000000
user overall point balance after offer 100000000000000000000000000
user1 overall point balance after offer 0
user balance after taker 99999900000000000000000000
user1 Balance after taker 99999948250000000000000000
stock points 1000
user balance after abort 99999900000000000000000000
user1 Balance after abort 99999948250000000000000000
user Refund balance After 50000000000000000000
user1 Refund Balance After 0
user claimable point balance 0
user1 claimable point balance 0
user overall point balance after 100000000000000000000000000
user1 overall point balance after 0

Impact

Users who submit takers on offers that abort have their collateral locked in the contract, unable to withdraw funds.

Tools Used

manual review
foundry

Recommendations

Consider using a better Precision in order to have the points scale to the division of the amount to avoid this issue.

one option is to implement the decimal scaler in the depositAmount calculation rather than the getDepositAmount itteration.

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points * Constants.COLLATERAL_RATE_DECIMAL_SCALER, // Apply the scaler here
preOfferInfo.amount,
Math.Rounding.Floor
);
function getDepositAmount(
OfferType _offerType,
uint256 _collateralRate,
uint256 _amount,
bool _isMaker,
Math.Rounding _rounding
) internal pure returns (uint256) {
if (_offerType == OfferType.Bid && _isMaker) {
return _amount;
}
​
if (_offerType == OfferType.Ask && !_isMaker) {
return _amount;
}
​
return Math.mulDiv(
_amount,
_collateralRate,
1, // No need for the scaler here
_rounding
);
}
Updates

Lead Judging Commences

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

finding-PreMarkets-abortBidTaker-amount-wrong-StockInfo-points

Valid high severity, due to incorrect computation of `depositAmount` within `abortBidTaker`, when aborting bid offers created by takers, the collateral refund will be completely wrong for the taker, and depending on the difference between the value of `points` and `amount`, it can possibly even round down to zero, causing definite loss of funds. If not, if points were worth less than the collateral, this could instead be used to drain the CapitalPool contract instead.

Support

FAQs

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