search

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

5 stars 3 forks source link

bareli - wrong implement of "_placeOrder" #22

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

bareli

medium

wrong implement of "_placeOrder"

Summary

we are comparing the same (order.comparison != -1)in if statement

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();

Impact

we are not implementing the if statement in the right way. we are comparing the same order.comparison not equal to -1.

Code Snippet

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

Tool used

Manual Review

Recommendation

if (order.comparison != -1)

sherlock-admin2 commented 5 months ago

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

z3s commented:

Invalid; No problem to system's functionality mentioned.

FSchmoede commented:

Not correct. It is checking for -1 and 1 (meaning lte or gte based on the TriggerOrder type).