code-423n4 / 2021-12-pooltogether-findings

0 stars 0 forks source link

Can delegate burnt tokens to oneself #66

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

cmichel

Vulnerability details

Failed signature verifications with ECDSA.recover return the zero address. The Ticket.delegateWithSignature function does not check that the provided _user is non-zero. This leads to the following attack which allows an attacker to delegate the zero address' balance (which is used as the burn address) to their own address.

POC

Impact

As the actual balance of the zero address (balanceOf(address0)) is never increased in a _burn and neither is a transfer to the zero address allowed using the OZ ERC20 _transfer implementation, this does luckily not pose a risk.

Recommended Mitigation Steps

We still recommend actively prohibiting _user to be the zero address in the delegateWithSignature call in case the delegates[0] field is used for anything in the future.

PierrickGT commented 2 years ago

I've acknowledged the issue cause we won't make this change during this audit but when we will redeploy the Ticket contract.

PierrickGT commented 2 years ago

After doing some tests in the following PR, we realized that this exploit is not possible. It will throw an error on this line of the OpenZeppelin ECDSA util contract: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol#L140 So for this reason, we disputed the issue. https://github.com/pooltogether/v4-core/pull/252