Missing Deadline in UniStaker.sol Stake Delegation Leads to Potential Misuse

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L315
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L382
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L512
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L544

Vulnerability details

Impact

The absence of a deadline check in the stakeMoreOnBehalf, stakeOnBehalf, withdrawOnBehalf, claimRewardOnBehalf functions could lead to unauthorized staking actions if signatures are reused or intentions change over time, potentially compromising the integrity of the staking process and user funds.

Proof of Concept

let's take a example of stakeMoreOnBehalf.
Generate a signature for stakeMoreOnBehalf without a deadline.

function stakeOnBehalf(
    uint256 _amount,
    address _delegatee,
    address _beneficiary,
    address _depositor,
    // @audit missing deadline
    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);
  }

Simulate a delay before using the signature to stake more on behalf of the user.

Observe that the signature remains valid indefinitely, allowing for potential misuse.

For Example:

Alice initially decides to increase her stake and provides Bob with a signature to do so.

Over the next 30 days, the market conditions change, and Alice decides not to add more to her stake.

Alice assumes the signature she gave Bob is no longer valid because too much time has passed.

However, Bob or a malicious actor who obtained the signature proceeds to call stakeMoreOnBehalf after the 30 days.

The UniStaker contract accepts the signature since there's no deadline mechanism in place.

The additional tokens are staked under Alice's deposit ID, contrary to her current investment strategy.

The lack of a deadline allows for the execution of an action that no longer aligns with the user's intent, demonstrating the vulnerability.

Tools Used

Manual Review

Recommended Mitigation Steps

Add expiration param to the following functions: stakeMoreOnBehalf, stakeOnBehalf, withdrawOnBehalf, claimRewardOnBehalf.

Assessed type

Error

[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