DefaultAccount.executeTransactionFromOutside does not validate if the signature is valid for the sender

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/DefaultAccount.sol#L130-L136

Vulnerability details

Impact

As it explains in the docs https://era.zksync.io/docs/dev/developer-guides/aa.html#iaccount-interface , the executeTransactionFromOutside function is highly encouraged, since there needs to be some way, in case of priority mode (e.g. if the operator is unresponsive), to be able to start transactions from your account from outside, which makes it be always possible to execute transactions from L1 for this account.

But the DefaultAccount.executeTransactionFromOutside implementation does not validate if the signature in the transaction is valid for the sender address. So in the above case, any transaction from L1 can be disguised as being sent from any EOA address.

Proof of Concept

DefaultAccount.executeTransactionFromOutside is very simple. It calls _validateTransaction and then calls _execute, and it does not check any return data from the functions.

    function executeTransactionFromOutside(
        Transaction calldata _transaction
    ) external payable override ignoreNonBootloader ignoreInDelegateCall {
        // The account recalculate the hash on its own
        _validateTransaction(bytes32(0), _transaction);
        _execute(_transaction);
    }

But the _validateTransaction function will NOT revert if the signature is invalid for address(this) . It returns a bytes4 magic, bytes4(0) denotes invalid signature, and the ACCOUNT_VALIDATION_SUCCESS_MAGIC denotes the signature is valid.

But executeTransactionFromOutside does not check the magic returned. So in the above case, any transaction from L1 can be disguised as being sent from any EOA address.

Tools Used

Manual review

Recommended Mitigation Steps

Check if the magic is valid.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #30

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

L

[c4-judge]

GalloDaSballo marked the issue as grade-c