Incorrect accounting in openPosition partially fill logic
Summary
The entire quote value is deducted from lockedBalances and partyBLockedBalances when the quote is only being partially filled.
Vulnerability Detail
In PartBFacetImpl openPosition() when a quote is being partially filled and newStatus != QuoteStatus.CANCELED the filledLockedValues are deducted from pendingLockedBalances and partyBPendingLockedBalances but later the entire quote value is added to lockedBalances and partyBLockedBalances, allowing a quote to be opened without being fully backed by the locked values of A and B. The locked values equivalent to quote.quantity - filledLockedValues have not been accounted for in the subtraction from pending locked balances and so are unbacked when the position is opened.
Impact
Party A and B receive a partial return of their locked balances and a position is opened with only part of what should be the locked balances allocated towards it, allowing both parties to partially enter into the position at no cost.
// @audit entire quote.quantity is added to locked balances
accountLayout.lockedBalances[quote.partyA].addQuote(quote);
accountLayout.partyBLockedBalances[quote.partyB][quote.partyA].addQuote(quote);`
Tool used
Manual Review
Recommendation
For all cases when the quote is partially filled (including when newStatus == QuoteStatus.CANCELED) pendingLockedBalances and partyBPendingLockedBalances should have the value filledLockedValues deducted; lockedBalances, partyBLockedBalances should be increased by filledLockedValues to maintain proper accounting.
nican0r
false
Incorrect accounting in openPosition partially fill logic
Summary
The entire quote value is deducted from lockedBalances and partyBLockedBalances when the quote is only being partially filled.
Vulnerability Detail
In PartBFacetImpl
openPosition()when a quote is being partially filled and newStatus != QuoteStatus.CANCELED the filledLockedValues are deducted from pendingLockedBalances and partyBPendingLockedBalances but later the entire quote value is added to lockedBalances and partyBLockedBalances, allowing a quote to be opened without being fully backed by the locked values of A and B. The locked values equivalent to quote.quantity - filledLockedValues have not been accounted for in the subtraction from pending locked balances and so are unbacked when the position is opened.Impact
Party A and B receive a partial return of their locked balances and a position is opened with only part of what should be the locked balances allocated towards it, allowing both parties to partially enter into the position at no cost.
Code Snippet
` if (newStatus == QuoteStatus.CANCELED) { // send trading Fee back to partyA LibQuote.returnTradingFee(currentId); accountLayout.pendingLockedBalances[quote.partyA].subQuote(quote); accountLayout.partyBPendingLockedBalances[quote.partyB][quote.partyA].subQuote( quote ); } else { // @audit filledLockedValues are subtracted accountLayout.pendingLockedBalances[quote.partyA].sub(filledLockedValues); accountLayout.partyBPendingLockedBalances[quote.partyB][quote.partyA].sub( filledLockedValues ); } newQuote.lockedValues = quote.lockedValues.sub(filledLockedValues); newQuote.initialLockedValues = newQuote.lockedValues; quote.quantity = filledAmount; quote.lockedValues = appliedFilledLockedValues;
// @audit entire quote.quantity is added to locked balances accountLayout.lockedBalances[quote.partyA].addQuote(quote); accountLayout.partyBLockedBalances[quote.partyB][quote.partyA].addQuote(quote);`
Tool used
Manual Review
Recommendation
For all cases when the quote is partially filled (including when newStatus == QuoteStatus.CANCELED) pendingLockedBalances and partyBPendingLockedBalances should have the value filledLockedValues deducted; lockedBalances, partyBLockedBalances should be increased by filledLockedValues to maintain proper accounting.
Duplicate of #226