sherlock-audit/2024-05-kwenta-x-perennial-integration-update-judging

bareli - wrong implementation of nonce in _placeOrder

Closed this issue · 1 comments

bareli

medium

wrong implementation of nonce in _placeOrder

Summary

we are implementing the ++latestNonce in our _orders.here we are increasing the latestNonce first then implementing the _orders.

Vulnerability Detail

function _placeOrder(
address account,
IMarket market,
TriggerOrder memory order
) internal isMarketInstance(market) {
if (order.fee.isZero()) revert MultiInvokerInvalidOrderError();
if (order.comparison != -1 && order.comparison != 1) revert MultiInvokerInvalidOrderError();
if (
order.side > 3 || // Invalid side
(order.side == 3 && order.delta.gte(Fixed6Lib.ZERO)) // Disallow placing orders that increase collateral
) revert MultiInvokerInvalidOrderError();

@>>    _orders[account][market][++latestNonce].store(order);
    emit OrderPlaced(account, market, latestNonce, order);
}

Impact

latestNonce will never be zero in _orders.we are increasing first then implementing.

Code Snippet

https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L438

Tool used

Manual Review

Recommendation

_orders[account][market][latestNonce++].store(order);

2 comment(s) were left on this issue during the judging contest.

z3s commented:

Low/Info; No problem to system's functionality mentioned.

FSchmoede commented:

It is not shown how this can be used as an attack vector.