code-423n4 / 2024-04-revert-mitigation-findings

1 stars 1 forks source link

H-01 MitigationConfirmed #76

Open c4-bot-5 opened 5 months ago

c4-bot-5 commented 5 months ago

Lines of code

Vulnerability details

C4 issue

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

Comments

Throughout the codebase, Revert relies on the Permit2 library to allow for transferring tokens via permits. The Permit2 API requires the Permit2 API-user to pass in a ERC20 token that will be used to transfer tokens. Revert does not apply any safety checks to this token and allows a user to pass in any token value they want. This results in a user tricking Revert into thinking that the whitelisted token was sent to Revert when it wasn't.

Mitigation

PR #19

Revert applies the same asset check throughout the V3Vault contract, which primarily utilizes the Permit2 library. The check was applied to three V3Vault functions (_repay(), liquidate(), and _deposit()):

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

As you can see above, the permitted.token is checked to equal asset, which represents the lent/borrowed token in the V3Vault.

Each check is added appropriately to the _repay(), _deposit(), and liquidate() functions. In each function, the safety check ensures the caller will send assets to the V3Vault.

Anything Else We Should Know

The V3Utils contract also utilizes Permit2. However, since the intention of the contract is not to store ERC20 tokens, there is no immediate risk of not checking if permit.permitted.token equals a specific token. Although users may manipulate the permit they pass to V3Utils, there is no major risk to either the protocol nor other users. Note that AutoCompound.sol does interact with V3Utils but does not pass in any permits. Based on how V3Utils is designed, if no permits are passed into the various swap functions, Permit2 will not be called.

Conclusion

LGTM

c4-judge commented 5 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 5 months ago

jhsagd76 marked the issue as confirmed for report