DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

In `_performForcedBid()` during primary margin call, `m.short.ercDebt` can be calculated less than actual, causing higher than expected `ercDebtSocialized`

t0x1c

Summary

Unsafe down-casting from uint256 to uint88 can lead to under-reporting of m.short.ercDebt inside _performForcedBid() while performing a primary margin call.

Vulnerability Details

The calculation inside _performForcedBid():

File: contracts/facets/MarginCallPrimaryFacet.sol
​
229 m.short.ercDebt = uint88(
230 m.ethDebt.div(_bidPrice.mul(1 ether + m.callerFeePct + m.tappFeePct))
231 ); // @dev(safe-cast)
232 uint96 ercDebtSocialized = ercDebtPrev - m.short.ercDebt;

The data types of these variables are:

uint88 ercDebt
uint256 ethDebt
uint80 _bidPrice
uint256 callerFeePct
uint256 tappFeePct

The unsafe casting of uint256 to uint88 can cause truncation of the value and hence the real ercDebt, as shown in the following PoC. This reduced m.short.ercDebt causes an increased ercDebtSocialized in Line 232.

PoC

Create a new file under test/ folder named MathCastingPerformForcedBid.t.sol and run the following code via forge test --mt test_casting_performForcedBid -vv:

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity 0.8.21;
​
import {U256, U88, U80} from "contracts/libraries/PRBMathHelper.sol";
import {console} from "contracts/libraries/console.sol";
​
contract MathCastingPerformForcedBid {
using U256 for uint256;
using U88 for uint88;
using U80 for uint80;
​
uint256 private ethDebt;
uint80 private bidPrice;
​
function setUp() public {
// we could easily keep the below value of `ethDebt` as `type(uint256).max`, but wanted to show that
// it starts truncating values way before that.
ethDebt = 1208925819614629174706175 * 0.000001 ether; // equivalent to `type(uint80).max * 0.000001 ether`
bidPrice = type(uint80).max;
}
​
/* solhint-disable no-console */
function test_casting_performForcedBid() public view {
uint256 ercDebt256 = (ethDebt.div(bidPrice.mul(1 ether + 0 + 0))); // assuming `m.callerFeePct = m.tappFeePct = 0` for a simpler calculation
uint88 ercDebt88 = uint88(ethDebt.div(bidPrice.mul(1 ether + 0 + 0)));
console.log("ercDebt256 =");
console.log(ercDebt256);
console.log("\nercDebt88 =");
console.log(ercDebt88);
}
}

Output:

Logs:
ercDebt256 =
1000000000000000000000000000000
​
ercDebt88 =
53933267234082950232408064

ercDebt256 is the actual value while ercDebt88 is the truncated one used by the protocol.

Impact

  • Higher ercDebtSocialized to other users and hence loss of funds for them.

  • This also throws off other calculations since lesser ercAmount is filled than expected via the forced bid.

Tools Used

Manual inspection, forge test.

Recommendations

It is recommended to specify a correct upper limit for these values using a require() statement inside the function, so that they do not eventually overflow on performing arithmetic operations but instead revert at the start itself.

Updates

Lead Judging Commences

0xnevi Lead Judge
about 2 years ago
0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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