sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

brgltd - Permit functionality breaking on token name update leading to potential loss of funds #21

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 1 month ago

brgltd

Medium

Permit functionality breaking on token name update leading to potential loss of funds

Summary

Updating the token name for SDAO.sol can cause the permit functionality to revert.

Vulnerability Detail

SDAO.sol contains a function file() to update the token name or symbol. Since permit() uses the EIP712 _DOMAIN_SEPARATOR with the token name (domain separator uses the cached value when block.id matches the block.id from the contract deployment), messages signed offchain with the new token name will not work.

In essence, updating a SubDAO token name can break the permit functionality and potentially cause negative repercussions on users who are providing offhchain approvals using the new token name which will mismatch the codebase _DOMAIN_SEPARATOR that will continue to use the old token name.

Impact

It's tricky to quantity loss of funds involved with this bug. However, there are scenarios where breaking permit for SubDAO tokens can lead to loss of funds for users. DeFi is made of highly composable money legos, and specially for an extremely popular protocol like MakerDAO, there can be downwards effects from permit breaking. For example:

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/SDAO.sol#L175-L176

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/SDAO.sol#L116

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/SDAO.sol#L129

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/SDAO.sol#L383

Tool used

Manual Review

Recommendation

When updating the token name on file(), _DOMAIN_SEPARATOR should be recalculated with the new token name.

sunbreak1211 commented 1 month ago

This is specified in the scope: "SDAO domain separator might be out of date when the token name and/or symbol changes."...