code-423n4 / 2023-05-base-findings

1 stars 0 forks source link

Possible inflation of ETH after EIP-4758 #104

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ethereum-optimism/optimism/blob/daaf917b201aae021fb10da03ef1262a13e00353/packages/contracts-bedrock/contracts/libraries/Burn.sol#L33-L42 https://github.com/ethereum-optimism/optimism/blob/daaf917b201aae021fb10da03ef1262a13e00353/packages/contracts-bedrock/contracts/L2/L2ToL1MessagePasser.sol#L80-L89

Vulnerability details

Impact

BASE/Optimism attempts to be EVM equivalent.

If EIP-4758 will be implemented in BASE - The mechanism for burning L2 ETH using selfdestruct will not work. This will lead to an inflation of ETH on L2 in regards to L1.

If EIP-4758 will not be implemented in BASE: Users will need to develop their smart contracts differently then on ethereum as selfdestruct will act differently between the chains. This will break the EVM equivalence.

Proof of Concept

Please read https://eips.ethereum.org/EIPS/eip-4758 which is on track to be added to ethereum. In breif, the selfdestruct code will not destroy the state of an account, but will continue to send the funds to the receiver (in this case itself).

L2 L2ToL1MessagePasser has a burn which is used to prevent the amount of ETH on L2 inflating when ETH is withdrawn https://github.com/ethereum-optimism/optimism/blob/daaf917b201aae021fb10da03ef1262a13e00353/packages/contracts-bedrock/contracts/L2/L2ToL1MessagePasser.sol#L80-L89

     * @notice Removes all ETH held by this contract from the state. Used to prevent the amount of
     *         ETH on L2 inflating when ETH is withdrawn. Currently only way to do this is to
     *         create a contract and self-destruct it to itself. Anyone can call this function. Not
     *         incentivized since this function is very cheap.
     */
    function burn() external {
        uint256 balance = address(this).balance;
        Burn.eth(balance);
        emit WithdrawerBalanceBurnt(balance);
    }

Burn.eth deploys a new contract with the specified ETH. The contract then selfdestructs to itself resulting in the ETH provided to "disappear" because of the deletion.

https://github.com/ethereum-optimism/optimism/blob/daaf917b201aae021fb10da03ef1262a13e00353/packages/contracts-bedrock/contracts/libraries/Burn.sol#L14-L42

    function eth(uint256 _amount) internal {
        new Burner{ value: _amount }();
    }
----------
/**
 * @title Burner
 * @notice Burner self-destructs on creation and sends all ETH to itself, removing all ETH given to
 *         the contract from the circulating supply. Self-destructing is the only way to remove ETH
 *         from the circulating supply.
 */
contract Burner {
    constructor() payable {
        selfdestruct(payable(address(this)));
    }
}

As can be seen from the snippets above, the impact stated the impact section will happen when EIP-4758 will be introduced.

Tools Used

Static analysis

Recommended Mitigation Steps

In order to support "deletion" of ETH on L2 to prevent inflation, consider adding a predeploy address that will be treated in op-geth as always having zero balance. Therefore, transferring/calling this address will "delete" the funds.

Assessed type

ETH-Transfer

c4-judge commented 1 year ago

0xleastwood marked the issue as primary issue

c4-judge commented 1 year ago

0xleastwood marked the issue as satisfactory

c4-judge commented 1 year ago

0xleastwood marked the issue as selected for report

anupsv commented 1 year ago

EIP not yet implemented. Not a vuln.

c4-sponsor commented 1 year ago

anupsv marked the issue as sponsor disputed

0xleastwood commented 1 year ago

Agree that this is assuming the EIP is implemented. Currently, Base protocol is unaffected. Downgrading to low risk.

c4-judge commented 1 year ago

0xleastwood changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xleastwood marked the issue as grade-b

c4-judge commented 1 year ago

0xleastwood marked the issue as not selected for report