adjust() may underflow resulting in can't settle

Summary

If the difference of invalidation is greater than the original value, it will result in adjust() underflow, the user will not be able to settle, and the collateral will be locked.

Vulnerability Detail

in adjust(), we'll adjust position based on the difference between latestPosition.invalidation and self.invalidation

    function adjust(Position memory self, Position memory latestPosition) internal pure {
@>      Invalidation memory invalidation = latestPosition.invalidation.sub(self.invalidation);
        (self.maker, self.long, self.short) = (
            UFixed6Lib.from(Fixed6Lib.from(self.maker).add(invalidation.maker)),
@>          UFixed6Lib.from(Fixed6Lib.from(self.long).add(invalidation.long)),
            UFixed6Lib.from(Fixed6Lib.from(self.short).add(invalidation.short))
        );
    }

This difference may be greater than the original value, resulting in adjust() underflow

Take the following example Suppose alice has the following Position (caused by orale delay during update()) t=100 long =100 invalidation = 0 (latestPosition) t=200 long = 50 invalidation = 0 (pending) t=300 long = 0 invalidation = 0 (pending)

Subsequent oralce submissions, assuming t=200 is an invalid oralce and t=300 oralce is valid

when settle : t=200 (oralce invalid) long = 100 invalidation = -50
when settle : t=300 (oralce valid) long = 0 + (-50) => **** underflow

POC

The following code demonstrates this scenario as described above

add to Market.test.ts

      context('invalidation', async () => {
...

        it('testdesync', async () => {
          const positionMaker = parse6decimal('2.000')
          const positionLong = parse6decimal('1.000')
          const collateral = parse6decimal('1000')

          const oracleVersion = {
            price: parse6decimal('100'),
            timestamp: TIMESTAMP,
            valid: true,
          }
          oracle.at.whenCalledWith(oracleVersion.timestamp).returns(oracleVersion)
          oracle.status.returns([oracleVersion, oracleVersion.timestamp + 100])
          oracle.request.returns()

          dsu.transferFrom.whenCalledWith(userB.address, market.address, collateral.mul(1e12)).returns(true)
          await market.connect(userB).update(userB.address, positionMaker, 0, 0, collateral, false)

          const oracleVersion2 = {
            price: parse6decimal('100'),
            timestamp: TIMESTAMP + 100,
            valid: true,
          }
          oracle.at.whenCalledWith(oracleVersion2.timestamp).returns(oracleVersion2)
          oracle.status.returns([oracleVersion2, TIMESTAMP + 100])
          oracle.request.returns()

          dsu.transferFrom.whenCalledWith(user.address, market.address, collateral.mul(1e12)).returns(true)
          await market.connect(user).update(user.address, 0, parse6decimal('1.000'), 0, collateral, false)

          oracle.at.whenCalledWith(oracleVersion2.timestamp).returns(oracleVersion2)
          oracle.status.returns([oracleVersion2, TIMESTAMP + 200])
          oracle.request.returns()

          await market.connect(user).update(user.address, 0, parse6decimal('0.500'), 0, 0, false)  

          oracle.at.whenCalledWith(oracleVersion2.timestamp).returns(oracleVersion2)
          oracle.status.returns([oracleVersion2, TIMESTAMP + 300])
          oracle.request.returns()

          await market.connect(user).update(user.address, 0, parse6decimal('0'), 0, 0, false)  

          // invalid oracle version
          const oracleVersion3 = {
            price: 0,
            timestamp: TIMESTAMP + 200,
            valid: false,
          }
          oracle.at.whenCalledWith(oracleVersion3.timestamp).returns(oracleVersion3)

          // next oracle version is valid
          const oracleVersion4 = {
            price: parse6decimal('100'),
            timestamp: TIMESTAMP + 300,
            valid: true,
          }
          oracle.at.whenCalledWith(oracleVersion4.timestamp).returns(oracleVersion4)

          // oracleVersion4 commited
          oracle.status.returns([oracleVersion4, oracleVersion4.timestamp + 100])
          oracle.request.returns()

          // reset to 0
          await market.connect(user).update(user.address, 0, 0, 0, 0, false)

        })
      })

$ yarn workspace @equilibria/perennial-v2 run test --grep "testdesync"

  Market
    already initialized
      #update
        invalidation

(reason="VM Exception while processing transaction: reverted with custom error 'UFixed6UnderflowError(-500000)'"

Impact

When adjust() revert, causing the user to be unable to settle , resulting in the collateral being locked

Code Snippet

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/Position.sol#L155-L157

Tool used

Manual Review

Recommendation

in adjust() If less than 0, take 0

sherlock-audit / 2023-09-perennial-judging

bin2chen - adjust() may underflow resulting in can't settle #26