yixxas - Wrong implementation in `_providerContract()`

[sherlock-admin]

yixxas

medium

Wrong implementation in _providerContract()

Summary

_providerContract() seems to be implemented wrongly. As a result, payoffType == CONTRACT cannot be used.

Vulnerability Detail

self.data is left shifted by 80 bits and then casted to bytes20 to recover the provider contract's address. This computation however, will never be able to return a valid ethereum address as the least significant 10 bits of the results will always be 0 regardless of the value of self.data. Since self.data is 30 bytes, I suppose self.data is meant to store the provider contract's address in the first 20 bytes, and 10 bytes of other data in the last 10 bytes.

  function _providerContract(
    PayoffDefinition memory self
  ) private pure returns (IContractPayoffProvider) {
    if (self.payoffType != PayoffType.CONTRACT) revert PayoffDefinitionNotContract(self.payoffType, self.data);
    // Shift to pull the last 20 bytes, then cast to an address
    return IContractPayoffProvider(address(bytes20(self.data << 80)));
  }

Impact

payoffType == CONTRACT cannot be used as price transformation is derived from _providerContract(), but is currently returning an incorrect address.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/interfaces/types/PayoffDefinition.sol#L81

Tool used

Manual Review

Recommendation

We should shift to the right instead.

  function _providerContract(
    PayoffDefinition memory self
  ) private pure returns (IContractPayoffProvider) {
    if (self.payoffType != PayoffType.CONTRACT) revert PayoffDefinitionNotContract(self.payoffType, self.data);
    // Shift to pull the last 20 bytes, then cast to an address
-   return IContractPayoffProvider(address(bytes20(self.data << 80)));
+   return IContractPayoffProvider(address(bytes20(self.data >> 80)));
  }