bin2chen - MultiInvoker is not backward compatible

[sherlock-admin2]

bin2chen

high

MultiInvoker is not backward compatible

Summary

TriggerOrder added interfaceFee2 and modified the old solt May cause old orders to use the wrong fees value

Vulnerability Detail

New version of TriggerOrder adds interfaceFee2. Also modified old interfaceFee.amount and interfaceFee.unwrap solt positions

Old version:

struct StoredTriggerOrder {
    /* slot 0 */
    uint8 side;                // 0 = maker, 1 = long, 2 = short, 3 = collateral
    int8 comparison;           // -2 = lt, -1 = lte, 0 = eq, 1 = gte, 2 = gt
    uint64 fee;                // <= 18.44tb
    int64 price;               // <= 9.22t
    int64 delta;               // <= 9.22t
@>  uint48 interfaceFeeAmount; // <= 281m

    /* slot 1 */
    address interfaceFeeReceiver;
@>  bool interfaceFeeUnwrap;
    bytes11 __unallocated0__;
}

New version：

struct StoredTriggerOrder {
    /* slot 0 */
    uint8 side;         // 0 = maker, 1 = long, 2 = short
    int8 comparison;    // -2 = lt, -1 = lte, 0 = eq, 1 = gte, 2 = gt
    uint64 fee;         // <= 18.44tb
    int64 price;        // <= 9.22t
    int64 delta;        // <= 9.22t
@>  bytes6 __unallocated0__;

    /* slot 1 */
    address interfaceFeeReceiver1;
@>  uint48 interfaceFeeAmount1;      // <= 281m
@>  bool interfaceFeeUnwrap1; 
    bytes5 __unallocated1__;

    /* slot 2 */
    address interfaceFeeReceiver2;
    uint48 interfaceFeeAmount2;      // <= 281m
    bool interfaceFeeUnwrap2;
    bytes5 __unallocated2__;
}

1. old interfaceFeeAmount is deprecated
2. interfaceFeeAmount1 occupies old interfaceFeeUnwrap

If there is an old order, the interfaceFee parsed after the update will be wrong Possible scenarios

1. interfaceFee1.amount is cleared to 0 because interfaceFeeAmount solt is deprecated.

2. the new interfaceFee1.amount will be very large if the old order has unwrap=true (interfaceFee1.amount uses the old unwrap position) interfaceFee1.amount= [unwrap+0000000000]= [0x010000000000] = ~1M fees (0x010000000000 / 1e6)

   Also unwrap becomes false

Impact

Old orders use the wrong fees value

Code Snippet

https://github.com/sherlock-audit/2024-02-perennial-v2-3/blob/main/perennial-v2/packages/perennial-extensions/contracts/types/TriggerOrder.sol#L19

Tool used

Manual Review

Recommendation

interfaceFee1 follows the old interfaceFee's solt position interfaceFee2 adds backward

struct StoredTriggerOrder {
    /* slot 0 */
    uint8 side;                // 0 = maker, 1 = long, 2 = short, 3 = collateral
    int8 comparison;           // -2 = lt, -1 = lte, 0 = eq, 1 = gte, 2 = gt
    uint64 fee;                // <= 18.44tb
    int64 price;               // <= 9.22t
    int64 delta;               // <= 9.22t
    uint48 interfaceFeeAmount; // <= 281m

    /* slot 1 */
    address interfaceFeeReceiver;
    bool interfaceFeeUnwrap;
    bytes11 __unallocated0__;

    /* slot 2 */
    address interfaceFeeReceiver2;
    uint48 interfaceFeeAmount2;      // <= 281m
    bool interfaceFeeUnwrap2;
    bytes5 __unallocated2__;
}

read()/store() modify accordingly

Duplicate of #12

[sherlock-admin2]

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

panprog commented:

valid high, dup of #12