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

H-01 MitigationConfirmed #7

Open
c4-bot-7 opened this issue Apr 22, 2024 · 1 comment
Open

H-01 MitigationConfirmed #7

c4-bot-7 opened this issue Apr 22, 2024 · 1 comment
Labels
edited-by-warden mitigation-confirmed MR-H-01 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@c4-bot-7
Copy link
Contributor

c4-bot-7 commented Apr 22, 2024

Lines of code

Vulnerability details

C4 issue

H-01: V3Vault.sol permit signature does not check receiving token address is USDC

Comment

The issues comes from the fact that whenever permit2.permitTransferFrom is used in V3Vault,
the original code doesn't check if the input token in permit is the same with the vault's asset token:

if (permitData.length > 0) {
                (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
                    abi.decode(permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
                permit2.permitTransferFrom(
                    permit, ISignatureTransfer.SignatureTransferDetails(address(this), assets), msg.sender, signature
                );
            }

This allows the attacker to input any ERC20 token and proceed without actually transferring asset tokens
to the vault.

Mitigation

PR #19
The code now checks if input token is the same with asset token every time permit2.permitTransferFrom is used:

if (permitData.length != 0) {
                (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
                    abi.decode(permitData, (ISignatureTransfer.PermitTransferFrom, bytes));

                if (permit.permitted.token != asset) {
                    revert InvalidToken();
                }

                permit2.permitTransferFrom(
                    permit, ISignatureTransfer.SignatureTransferDetails(address(this), assets), msg.sender, signature
                );
            }

The mitigation resolved the original issue.

Conclusion

LGTM

@c4-judge
Copy link

c4-judge commented May 1, 2024

jhsagd76 marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
edited-by-warden mitigation-confirmed MR-H-01 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants