Skip to content

Audit Fix L04: Unauthorized deposit of WETH-like tokens on behalf of user #283

@ZenGround0

Description

@ZenGround0

Audit reference: [FIL-1132b525-L04]

From audit

The depositWithPermit function does not validate that msg.sender matches the permit signer (to address), unlike the depositWithPermitAndApproveOperator variants which use the validateSignerIsRecipient modifier. This allows anyone to call depositWithPermit on behalf of any user by using the user’s permit signature.
When used with tokens like WETH (Wrapped ETH) or similar tokens that have empty fallback function, an attacker can forge a user’s permit and deposit tokens into the user’s account
without the user’s direct authorization for that specific transaction.

I am not sure if we should fix this via the recommendation:

Add the validateSignerIsRecipient(to) modifier to the depositWithPermit function, consistent with the depositWithPermitAndApproveOperator variants. This will ensure that only the permit signer can execute their own permit, preventing third parties from executing permits on behalf of users.

My understanding was that the point of these permits was at least in part to allow for these deposit steps to be delegated to parties that send messages on behalf of other parties. And this issue is a reasonable tradeoff if we are bought into such behavior. I would like us to wholistically investigate our goals and the current state before making depositWithPermit match depositWithPermitAndApproveOperator. I am pretty sure something is suboptimal however because both these methods should have the same behavior.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    🐱 Todo

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions