Missing Expiry in Signature Verification in the UniStaker contracts

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L391
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L322
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L432
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L475
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L521
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L545
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L803

Vulnerability details

Impact

The absence of an expiry timestamp in the signature verification process can lead to potential security vulnerabilities. Malicious actors may delay the usage of certain signatures (and the corresponding operations) to their advantages.

Proof of Concept

In the stakeOnBehalf function, a signature is used to authorize staking on behalf of another user. However, the signature does not include an expiry timestamp, allowing it to be deliberately delayed:

solidityCopy codefunction stakeOnBehalf(
  uint256 _amount,
  address _delegatee,
  address _beneficiary,
  address _depositor,
  bytes memory _signature
) external returns (DepositIdentifier _depositId) {
  _revertIfSignatureIsNotValidNow(
    _depositor,
    _hashTypedDataV4(
      keccak256(
        abi.encode(
          STAKE_TYPEHASH, _amount, _delegatee, _beneficiary, _depositor, _useNonce(_depositor)
        )
      )
    ),
    _signature
  );
  _depositId = _stake(_depositor, _amount, _delegatee, _beneficiary);
}

Similar issues can be found in the other mentioned functions.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Introduce an expiry timestamp as part of the data signed by the user. This timestamp should be checked against the current block timestamp to ensure that the signature is still valid at the time of execution. If the signature has expired, the transaction should be reverted.

Modify the data structure for signing to include an expiry timestamp:

solidityCopy code// Updated type hash with expiry timestamp
bytes32 public constant STAKE_TYPEHASH = keccak256(
  "Stake(uint256 amount,address delegatee,address beneficiary,address depositor,uint256 nonce,uint256 expiry)"
);

// Updated stakeOnBehalf function with expiry check
function stakeOnBehalf(
  uint256 _amount,
  address _delegatee,
  address _beneficiary,
  address _depositor,
  uint256 _expiry,
  bytes memory _signature
) external returns (DepositIdentifier _depositId) {
  require(block.timestamp <= _expiry, "Signature expired");
  _revertIfSignatureIsNotValidNow(
    _depositor,
    _hashTypedDataV4(
      keccak256(
        abi.encode(
          STAKE_TYPEHASH, _amount, _delegatee, _beneficiary, _depositor, _useNonce(_depositor), _expiry
        )
      )
    ),
    _signature
  );
  _depositId = _stake(_depositor, _amount, _delegatee, _beneficiary);
}

Apply similar changes to the other functions that use signatures for authorization.

Assessed type

Access Control

[c4-judge]

MarioPoneder marked the issue as duplicate of #69

[c4-judge]

MarioPoneder marked the issue as not a duplicate

[c4-judge]

MarioPoneder marked the issue as duplicate of #205

[c4-judge]

MarioPoneder changed the severity to QA (Quality Assurance)

[c4-judge]

MarioPoneder marked the issue as grade-c

[c4-judge]

This previously downgraded issue has been upgraded by MarioPoneder

[c4-judge]

MarioPoneder marked the issue as satisfactory