UniStaker contract uses signature that do not have expiration

[c4-bot-6]

Lines of code

https://github.com/uniswapfoundation/UniStaker/blob/d745dd2a393f4b6a35bca2fd72f4cd198840c081/src/UniStaker.sol#L324

Vulnerability details

Impact

When a user creates a signature operation for the UniStaker contract there is no any expiration time for this operation. That means that once, the signer has signed the signature for the user it can use it for the end of life.

Proof of Concept

All UniStaker method that use signatures do not make use of an expiration parameter

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#L423
https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L466
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

like is done in UNI token implementation in the methods delegateBySig or permit:

Tools Used

Manual review

Recommended Mitigation Steps

Add expiration parameter to the signature and the function using the signatures. Do not forget to also make use of the expiration parameter like in this example :

require(now <= expiry, "Uni::delegateBySig: signature expired"); // 'now' was deprecated in solidity 0.7.0 better to use block.timestamp in newer applications

for example here is a sample implementation for the UniStaker#withdrawOnBehalf method:

    function withdrawOnBehalf(
        DepositIdentifier _depositId,
        uint256 _amount,
+       uint256 _expiry,
        address _depositor,
        bytes memory _signature
    ) external {
+       if (block.timestamp > _expiry) revert SignatureIsExpired();
        ...
    }

Assessed type

Other

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

This previously downgraded issue has been upgraded by MarioPoneder

[c4-judge]

MarioPoneder marked the issue as satisfactory