Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SECURITY] Non-Compliance : Incorrect Calculation of GTxDataNonZero and GTxDataZero #1363

Open
mhoste51 opened this issue Nov 29, 2024 · 0 comments

Comments

@mhoste51
Copy link


Our team at FuzzingLabs discovered a non-compliance in the gas GTxDataNonZero and GTxDataZero calculation.

Overview

The Ethereum Yellow Paper specifies gas costs for transaction data based on its content:

  1. GTxDataNonZero = 16:

    This gas cost is applied to each non-zero byte in the transaction data (calldata). Non-zero bytes are more expensive to process because they increase the complexity of state computation and transmission.

  2. GTxDataZero = 4:

    This gas cost is applied to each zero byte in the transaction data (calldata). Zero bytes are less expensive to process because they require fewer computational resources.

These costs are part of the total transaction gas cost and are essential for properly accounting for the computational resources required to handle transaction data.


Issue Observed in LEVM

LEVM fails to correctly calculate GTxDataNonZero and GTxDataZero in certain scenarios, particularly during transactions involving contract creation (CREATE or CREATE2).

Details:

  • During a CREATE operation, the bytecode (contract input) contains the actual data that should be used to compute the gas cost for GTxDataNonZero and GTxDataZero.
  • However, in LEVM, the gas cost calculation for transaction data (tx_calldata) incorrectly passes the calldata instead of the bytecode.
  • Since calldata is empty during a CREATE transaction, the calculated gas cost for data is 0, regardless of the actual content of the bytecode.

Code line :

let calldata_cost =
    gas_cost::tx_calldata(&initial_call_frame.calldata).map_err(VMError::OutOfGas)?;

Expected Behavior:

  • The gas cost for transaction data during a CREATE transaction should be based on the bytecode provided as input, not the calldata :
let calldata_cost =
    gas_cost::tx_calldata(&initial_call_frame.bytecode).map_err(VMError::OutOfGas)?;

Impact of Non-Compliance

  1. Underestimation of Gas Costs:
    • The gas consumed during a CREATE transaction is significantly underestimated, as the bytecode is not properly accounted for.
  2. Deviation from Ethereum Specifications:
    • This behavior violates the Ethereum Yellow Paper's rules for transaction gas costs, specifically the handling of GTxDataNonZero and GTxDataZero.
  3. Potential Exploits:
    • Attackers could exploit this miscalculation to deploy contracts with large and complex bytecode at a lower-than-expected gas cost, gaining an unfair advantage.

Recommendations for Fixing the Issue

  1. Correct Argument for Gas Cost Calculation:
    • Ensure that the gas cost calculation for transaction data (txcall) uses the bytecode (input) instead of the calldata during CREATE transactions.

Step to reproduce

Add to test :

#[test]
fn test_non_compliance_gas_cost_GTxDataNonZero_GTxDataZero() {
    let mut vm = new_vm_with_bytecode(Bytes::copy_from_slice(&[0x5f]
    ))
    .unwrap();
    vm.tx_kind = TxKind::Create;
    vm.env.gas_price = U256::zero();
    vm.env.gas_limit = U256::from(100_000_000);
    let res = vm.transact().unwrap();
    assert_eq!(res.gas_used, 53020);
}

Backtrace

thread 'test_non_compliance_gas_cost_GTxDataNonZero_GTxDataZero' panicked at crates/vm/levm/tests/edge_case_tests.rs:20:5:
assertion `left == right` failed
  left: 53004
 right: 53020
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8adb4b30f40e6fbd21dc1ba26c3301c7eeb6de3c/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/8adb4b30f40e6fbd21dc1ba26c3301c7eeb6de3c/library/core/src/panicking.rs:76:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: edge_case_tests::test_non_compliance_gas_cost_GTxDataNonZero_GTxDataZero
             at ./tests/edge_case_tests.rs:20:5
   5: edge_case_tests::test_non_compliance_gas_cost_GTxDataNonZero_GTxDataZero::{{closure}}
             at ./tests/edge_case_tests.rs:12:61
   6: core::ops::function::FnOnce::call_once
             at /home/mhoste/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/8adb4b30f40e6fbd21dc1ba26c3301c7eeb6de3c/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

failures:
    test_non_compliance_gas_cost_GTxDataNonZero_GTxDataZero

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 27 filtered out; finished in 0.12s
github-merge-queue bot pushed a commit that referenced this issue Dec 5, 2024
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->
- `number_of_words` is now calculated in a more performant way, without
iterating calldata.
- The initial_call_frame in Create is now initialized with empty
bytecode and the corresponding calldata send in the transaction. ->
After validations the calldata will be assigned to the bytecode and it
will be erased. This is mostly for using calldata for calculating some
gas costs.

<!-- Link to issues: Resolves #111, Resolves #222 -->

Closes #1362, #1363 

Running tests:
- ✓ Summary: 1547/4100 (37.73)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant