Signature Replay Vulnerability in TimelockTokenPool

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/team/TimelockTokenPool.sol#L168-L173

Vulnerability details

Impact

TimelockTokenPool contract has two withdrawal functionality, one that allows withdrawal to self and another that allows withdrawal to a different address.
The latter allows withdrawal to other addresses via a signature, it works by providing a signature and a receiving address on call, on the signature recovery, the returned signer acts as the recipient whose grants will be sent to the receiving _to address.

    function withdraw(address _to, bytes memory _sig) external {
        if (_to == address(0)) revert INVALID_PARAM();
        bytes32 hash = keccak256(abi.encodePacked("Withdraw unlocked Taiko token to: ", _to));
        address recipient = ECDSA.recover(hash, _sig);
        _withdraw(recipient, _to);
    }

The problem here is:

• Due to the lack of signature expiration or the use of a nonce value, the receiving address can continuously re-use the same signature to withdraw the recipient's accumulated grant before he can.

To Illustrate:

Consider the scenario where Alice, entitled to 200 grants, initially withdraws 100 for herself. She later decides to distribute 50 grants each to Bob and Charlie. Alice then signs a transaction to withdraw her current allowance of 50 grants to Bob. However, days later, when she intends to repeat the process for Charlie, Bob reuses Alice's previous transaction signature, to redirect her intended grant withdrawal to himself, leaving Charlie with either nothing or only a fraction of the intended amount, depending on Alice's remaining allowance at the time of execution.
Bob can continually reuse the same signature to withdraw Alice's grants whenever she's allocated any in the future

• Final thoughts

From the current implementation, I believe whenever the recipient wishes to withdraw to self, she uses TimelockTokenPool::withdraw() and TimelockTokenPool::withdraw(_to, sig) when she intends to send her withdrawal to a different address. The receiving address could be owned or it might not, but regardless the signature should only be usable once to prevent this issue.
Since the use of this functionality potentially puts the recipient's grants at risk of being stolen, I have labelled this issue to be of a High severity

Proof of Concept

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/team/TimelockTokenPool.sol#L168-L173

Tools Used

Manual Review

Recommended Mitigation Steps

Either add a requirement that ensures the caller is the returned recipient or integrate a nonce value for signatures per recipient.

Assessed type

Error

[c4-pre-sort]

minhquanym marked the issue as duplicate of #60

[c4-judge]

0xean marked the issue as satisfactory