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

Maker loses fund due to different rounding direction during creating and closing an offer

fyamf

Summary

When a maker creates an ask offer, the deposit amount is rounded up.
When a maker closes an ask offer, the refunded amount is rounded down.

The difference between them can lead to loss of fund in case of low decimal collateral tokens.

Vulnerability Details

When maker creates an ask offer, the transfer amount is rounded up:

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

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L92

return
Math.mulDiv(
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/libraries/OfferLibraries.sol#L44

If this offer is closed, the transferred amount is added to the balance of the maker as a MakerRefund to be able to withdraw it later. The amount is rounded down:

tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
refundAmount
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L450

Math.mulDiv(
_amount - usedAmount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
Math.Rounding.Floor
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/libraries/OfferLibraries.sol#L86

It shows that the maker loses some amount by just creating and closing an ask offer due to the rounding.

Please note that the decimal of the token (i.e. the collateral token) is unknown to us, and it depends on the admin that which tokens will be whitelisted. So, if the whitelisted token has low number of decimals, this rounding issue will have more impact on the makers.

For example two standard erc20 tokens with zero decimals are:
https://etherscan.io/address/0xaec2e87e0a235266d9c5adc9deb4b2e29b54d009#code
https://etherscan.io/token/0x1da4858ad385cc377165a298cc2ce3fce0c5fd31#code

Moreover, there is no mechanism in the protocol to withdraw these stuck fund in the protocol.

PoC

In this test Alice (maker) creates an ask offer to sell 1000 points for 1 collateral token (assuming that the decimal of the collateral token is small in order of 0, 1, 2). Because of rounding up, Alice should transfer 2 collateral token. Then, Alice closes her offer. Due to rounding down, Alice only receives 1 collateral token into her balance. So, she loses 1 collateral token.

function test_rounding() public {
address Alice = vm.addr(10); // maker
​
////// Alice(maker) creates a bid offer
vm.startPrank(Alice);
deal(address(mockUSDCToken), Alice, 2);
​
console.log(
"Alice balance of collateral token before: ",
mockUSDCToken.balanceOf(Alice)
);
​
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
1,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
// Alice closes her offer
preMarktes.closeOffer(stockAddr, offerAddr);
​
vm.stopPrank();
////////////////
​
console.log(
"Alice balance of USDC in the protocol: ",
tokenManager.userTokenBalanceMap(
address(Alice),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
)
);
}

The output is:

Logs:
Alice balance of collateral token before: 2
Alice balance of collateral token in the protocol: 1

Impact

  • Loss of fund due to non-similar rounding direction during creating and closing an offer.

Tools Used

Recommendations

The direction of rounding for this case should be similar.

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

Appeal created

fyamf Submitter
10 months ago
0xnevi Lead Judge
9 months ago
0xnevi Lead Judge 9 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.