DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

MIssing check in `sumReturnUint256` leading to possible underflow in MarketUtils.sol

Summary

In MarketUtils.sol the function sumReturnUint256 take in a uint256 number and an int256 number and returns their sum in uint256 but this function is not properly written as in a situation where the int256 number is more negative than the uint256 number is positive, an underflow will occur reverting the entire transaction.

Vulnerability Details

When checking if a Positions collateral is sufficient the function getMinCollateralFactorForOpenInterest() is used to get the minimum collateral factor, this function calls sumReturnUint256() to return the sum of a uint256 number and an int256 number in uint256

// @dev get the min collateral factor for open interest
// @param dataStore DataStore
// @param market the market to check
// @param longToken the long token of the market
// @param shortToken the short token of the market
// @param openInterestDelta the change in open interest
// @param isLong whether it is for the long or short side
function getMinCollateralFactorForOpenInterest(
IDataStore dataStore,
MarketProps memory market,
int256 openInterestDelta,
bool isLong
) internal view returns (uint256) {
uint256 openInterest = getOpenInterest(dataStore, market, isLong);
@> openInterest = sumReturnUint256(openInterest, openInterestDelta);
uint256 multiplierFactor = getMinCollateralFactorForOpenInterestMultiplier(dataStore, market.marketToken, isLong);
return applyFactor(openInterest, multiplierFactor);
}

In sumReturnUint256() if b is neagtive, it is converted to a positive uint256 and then subtracted from a.

function sumReturnUint256(uint256 a, int256 b) internal pure returns (uint256) {
if (b > 0) {
return a + uint256(b);
}
@> return a - uint256(-b);
}

But in a situation in which the int256 number is more negative than the uint256 number is positive, the function will revert due to underflow.

To explain better,lets take for example

a = 5

b = -7

when sumReturnUint256() is called

-(-7) is done converting b to 7

but then 5-7 is then attempted causing a revert due to underflow.

Impact

possible underflow in sumReturnUint256() as the function doesn't account for a situation where after conversion to positive b becomes greater than a

Tools Used

Manual Review

Recommendations

sumReturnUint256() should be corrected to return zero instead, to prevent underflows.

function sumReturnUint256(uint256 a, int256 b) internal pure returns (uint256) {
if (b > 0) {
return a + uint256(b);
}
-- return a - uint256(-b);
++ if (uint256(-b) > a) {
++ return 0;
++ } else {
++ return a - uint256(-b);
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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