The on behalf functions currently do not include a deadline parameter

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/491c7f63e5799d95a181be4a978b2f074dc219a5/src/UniStaker.sol#L327

Vulnerability details

Bug Description

In the UniStaker smart contract, there is a stakeOnBehalf function and other functions that can be called on behalf of a user. If a user wants UNI to be staked for them from another address, they sign a message and pass the generated signature, allowing the other address to call the stakeOnBehalf function. The issue is that the generated signature is perpetual and can be used at any time.

Proof-of-Concept

Alice (EOA) trusts Bob (EOA) and provides a signature for Bob to call the stakeOnBehalf function and pre-approve the UniStaker smart contract. After that, she asks Bob not to send the transaction because she changed her mind.
UniStaker.sol#L391-L399

function 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);
  }

Six months pass, and Bob becomes malicious. Bob can stake on behalf of Alice even after six months because the signature for EOA is not revocable.

Impact

If a user provides a signature allowing someone else to stake on their behalf using the UniStaker smart contract but later decides not to stake, there is a risk that the other party could still use that signature to stake on behalf of the user at any time in the future.

Tools Used

Manual

Recommended Mitigation Steps

Consider adding a deadline value to STAKE_TYPEHASH and other type hashes related to onBehalf calls:

bytes32 public constant STAKE_TYPEHASH = keccak256(
-    "Stake(uint256 amount,address delegatee,address beneficiary,address depositor,uint256 nonce)"
+    "Stake(uint256 amount,address delegatee,address beneficiary,address depositor,uint256 nonce,uint256 deadline)"
  );
  
function stakeOnBehalf(
    uint256 _amount,
+   uint256 _deadline,
    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)
+            STAKE_TYPEHASH, _amount, _delegatee, _beneficiary, _depositor, _useNonce(_depositor), _deadline
          )
        )
      ),
      _signature
    );
    _depositId = _stake(_depositor, _amount, _delegatee, _beneficiary);
  }

Additionally, ensure that the block.timestamp value is less than the current _deadline.

Assessed type

Context

[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-b

[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