Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The signature used in the TimelockTokenPool.withdraw function might be reused since the signed message doesn't include nonce and deadline #204

Closed
c4-bot-5 opened this issue Mar 26, 2024 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-60 🤖_60_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@c4-bot-5
Copy link
Contributor

Lines of code

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

Vulnerability details

The TimelockTokenPool contract allows for token withdrawals on behalf of users by utilizing a permit-like signature:

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

This function might be called multiple times as the tokens are unlocked gradually. The issue arises from the fact that the signed message doesn't include deadline and nonce. It means that anyone can reuse this signature and replay a withdrawal transaction.

It's not much of an issue if the user manages approvals meticulously, but users often opt for unlimited approvals to save on gas. In this case, an attacker is still unable to steal anything, but they can replay a withdrawal transaction without the user's consent. This is quite concerning, as this transaction might essentially involve swapping costToken for taikoToken and the user might have some other use for costToken in mind. There also might be extreme cases where the user attempts to prevent liquidation using costToken, only to be front-run by the attacker.

This issue becomes even more severe if the _to address is compromised. In such a scenario, an attacker can get taikoToken at the expense of the user. The issue becomes more likely because of the following:

  • Users might not think that the signature produced some time ago can be reused since such kinds of permits usually include nonce and deadline and can't be reused;
  • Users have no means to revoke the signature, and the only way to protect themselves from this attack is to execute both costToken approval and withdrawal within a single transaction which is possible but isn't simple and obvious for the common user.

Impact

An attacker can reuse a signature to execute an action without the user's consent. In some cases it allows the attacker to steal user's funds.

Proof of Concept

-

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding a nonce and deadline to the signed message. Additionally, consider exposing a function that marks the current nonce as used, providing a means for the user to revoke signatures.

Assessed type

Invalid Validation

@c4-bot-5 c4-bot-5 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Mar 26, 2024
c4-bot-5 added a commit that referenced this issue Mar 26, 2024
@c4-bot-11 c4-bot-11 added the 🤖_60_group AI based duplicate group recommendation label Mar 27, 2024
@c4-pre-sort
Copy link

minhquanym marked the issue as duplicate of #60

@c4-judge
Copy link
Contributor

0xean marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Apr 10, 2024
@c4-judge
Copy link
Contributor

0xean changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-60 🤖_60_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

4 participants