diff --git a/CHANGELOG.md b/CHANGELOG.md index db2105e..7f96e28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,12 @@ Supported `polkadot-sdk` rev: `unstable2507` - Standard JSON mode: Don't forward EVM bytecode related output selections to solc. - The supported `polkadot-sdk` release is `unstable2507`. - The `INVALID` opcode and OOB memory accesses now consume all remaining gas. +- Emit the `call_evm` and `delegate_call_evm` syscalls for contract calls. ### Fixed: - The missing `STOP` instruction at the end of `code` blocks. - The missing bounds check in the internal sbrk implementation. +- The call gas is no longer ignored. ## v0.5.0 diff --git a/crates/integration/contracts/CallGas.sol b/crates/integration/contracts/CallGas.sol new file mode 100644 index 0000000..0321c1d --- /dev/null +++ b/crates/integration/contracts/CallGas.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8; + +// Use a non-zero call gas that works with call gas clipping but not with a truncate. + +/* runner.json +{ + "differential": true, + "actions": [ + { + "Upload": { + "code": { + "Solidity": { + "contract": "Other" + } + } + } + }, + { + "Instantiate": { + "code": { + "Solidity": { + "contract": "CallGas" + } + }, + "data": "1000000000000000000000000000000000000000000000000000000000000001" + } + } + ] +} +*/ + +contract Other { + address public last; + uint public foo; + + fallback() external { + last = msg.sender; + foo += 1; + } +} + +contract CallGas { + constructor(uint _gas) payable { + Other other = new Other(); + address(other).call{ gas: _gas }(hex""); + assert(other.last() == address(this)); + } +} diff --git a/crates/integration/contracts/Send.sol b/crates/integration/contracts/Send.sol index 470592b..c8a3eb9 100644 --- a/crates/integration/contracts/Send.sol +++ b/crates/integration/contracts/Send.sol @@ -23,11 +23,6 @@ pragma solidity ^0.8; "data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a" } }, - { - "VerifyCall": { - "success": true - } - }, { "Call": { "dest": { @@ -36,11 +31,6 @@ pragma solidity ^0.8; "data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000001" } }, - { - "VerifyCall": { - "success": false - } - }, { "Call": { "dest": { @@ -48,11 +38,6 @@ pragma solidity ^0.8; }, "data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000000" } - }, - { - "VerifyCall": { - "success": false - } } ] } diff --git a/crates/integration/contracts/Transfer.sol b/crates/integration/contracts/Transfer.sol index 02ee701..412f4f5 100644 --- a/crates/integration/contracts/Transfer.sol +++ b/crates/integration/contracts/Transfer.sol @@ -23,11 +23,6 @@ pragma solidity ^0.8; "data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a" } }, - { - "VerifyCall": { - "success": true - } - }, { "Call": { "dest": { @@ -36,11 +31,6 @@ pragma solidity ^0.8; "data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000001" } }, - { - "VerifyCall": { - "success": false - } - }, { "Call": { "dest": { @@ -48,11 +38,6 @@ pragma solidity ^0.8; }, "data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000000" } - }, - { - "VerifyCall": { - "success": false - } } ] } diff --git a/crates/integration/src/tests.rs b/crates/integration/src/tests.rs index 08293b3..d559fb5 100644 --- a/crates/integration/src/tests.rs +++ b/crates/integration/src/tests.rs @@ -66,6 +66,7 @@ test_spec!(add_mod_mul_mod, "AddModMulModTester", "AddModMulMod.sol"); test_spec!(memory_bounds, "MemoryBounds", "MemoryBounds.sol"); test_spec!(selfdestruct, "Selfdestruct", "Selfdestruct.sol"); test_spec!(clz, "CountLeadingZeros", "CountLeadingZeros.sol"); +test_spec!(call_gas, "CallGas", "CallGas.sol"); fn instantiate(path: &str, contract: &str) -> Vec { vec![Instantiate { @@ -451,50 +452,6 @@ fn ext_code_size() { .run(); } -#[test] -#[should_panic(expected = "ReentranceDenied")] -fn send_denies_reentrancy() { - let value = 1000; - Specs { - actions: vec![ - instantiate("contracts/Send.sol", "Send").remove(0), - Call { - origin: TestAddress::Alice, - dest: TestAddress::Instantiated(0), - value, - gas_limit: None, - storage_deposit_limit: None, - data: Contract::send_self(U256::from(value)).calldata, - }, - ], - differential: false, - ..Default::default() - } - .run(); -} - -#[test] -#[should_panic(expected = "ReentranceDenied")] -fn transfer_denies_reentrancy() { - let value = 1000; - Specs { - actions: vec![ - instantiate("contracts/Transfer.sol", "Transfer").remove(0), - Call { - origin: TestAddress::Alice, - dest: TestAddress::Instantiated(0), - value, - gas_limit: None, - storage_deposit_limit: None, - data: Contract::transfer_self(U256::from(value)).calldata, - }, - ], - differential: false, - ..Default::default() - } - .run(); -} - #[test] fn create2_salt() { let salt = U256::from(777); diff --git a/crates/llvm-context/src/polkavm/evm/call.rs b/crates/llvm-context/src/polkavm/evm/call.rs index fc2c303..9760e8a 100644 --- a/crates/llvm-context/src/polkavm/evm/call.rs +++ b/crates/llvm-context/src/polkavm/evm/call.rs @@ -4,9 +4,8 @@ use inkwell::values::BasicValue; use crate::polkavm::context::Context; -const STATIC_CALL_FLAG: u32 = 0b0001_0000; -const REENTRANT_CALL_FLAG: u32 = 0b0000_1000; -const SOLIDITY_TRANSFER_GAS_STIPEND_THRESHOLD: u64 = 2300; +const STATIC_CALL_FLAG: u64 = 0b0001_0000; +const REENTRANT_CALL_FLAG: u64 = 0b0000_1000; /// Translates a contract call. pub fn call<'ctx>( @@ -38,33 +37,12 @@ pub fn call<'ctx>( let output_length_pointer = context.build_alloca_at_entry(context.xlen_type(), "output_length"); context.build_store(output_length_pointer, output_length)?; - let (flags, deposit_limit_value) = if static_call { - let flags = REENTRANT_CALL_FLAG | STATIC_CALL_FLAG; - ( - context.xlen_type().const_int(flags as u64, false), - context.word_type().const_zero(), - ) + let flags = if static_call { + REENTRANT_CALL_FLAG | STATIC_CALL_FLAG } else { - call_reentrancy_heuristic(context, gas, input_length, output_length)? + REENTRANT_CALL_FLAG }; - let deposit_pointer = context.build_alloca_at_entry(context.word_type(), "deposit_pointer"); - context.build_store(deposit_pointer, deposit_limit_value)?; - - let flags_and_callee = revive_runtime_api::calling_convention::pack_hi_lo_reg( - context.builder(), - context.llvm(), - flags, - address_pointer.to_int(context), - "address_and_callee", - )?; - let deposit_and_value = revive_runtime_api::calling_convention::pack_hi_lo_reg( - context.builder(), - context.llvm(), - deposit_pointer.to_int(context), - value_pointer.to_int(context), - "deposit_and_value", - )?; let input_data = revive_runtime_api::calling_convention::pack_hi_lo_reg( context.builder(), context.llvm(), @@ -85,10 +63,10 @@ pub fn call<'ctx>( .build_runtime_call( name, &[ - flags_and_callee.into(), - context.register_type().const_all_ones().into(), - context.register_type().const_all_ones().into(), - deposit_and_value.into(), + context.xlen_type().const_int(flags, false).into(), + address_pointer.to_int(context).into(), + value_pointer.to_int(context).into(), + clip_call_gas(context, gas)?, input_data.into(), output_data.into(), ], @@ -111,7 +89,7 @@ pub fn call<'ctx>( pub fn delegate_call<'ctx>( context: &mut Context<'ctx>, - _gas: inkwell::values::IntValue<'ctx>, + gas: inkwell::values::IntValue<'ctx>, address: inkwell::values::IntValue<'ctx>, input_offset: inkwell::values::IntValue<'ctx>, input_length: inkwell::values::IntValue<'ctx>, @@ -132,18 +110,6 @@ pub fn delegate_call<'ctx>( let output_length_pointer = context.build_alloca_at_entry(context.xlen_type(), "output_length"); context.build_store(output_length_pointer, output_length)?; - let deposit_pointer = context.build_alloca_at_entry(context.word_type(), "deposit_pointer"); - context.build_store(deposit_pointer, context.word_type().const_all_ones())?; - - let flags = context.xlen_type().const_int(0u64, false); - - let flags_and_callee = revive_runtime_api::calling_convention::pack_hi_lo_reg( - context.builder(), - context.llvm(), - flags, - address_pointer.to_int(context), - "address_and_callee", - )?; let input_data = revive_runtime_api::calling_convention::pack_hi_lo_reg( context.builder(), context.llvm(), @@ -164,10 +130,9 @@ pub fn delegate_call<'ctx>( .build_runtime_call( name, &[ - flags_and_callee.into(), - context.register_type().const_all_ones().into(), - context.register_type().const_all_ones().into(), - deposit_pointer.to_int(context).into(), + context.xlen_type().const_int(0u64, false).into(), + address_pointer.to_int(context).into(), + clip_call_gas(context, gas)?, input_data.into(), output_data.into(), ], @@ -201,82 +166,22 @@ pub fn linker_symbol<'ctx>( context.build_load_address(context.get_global(path)?.into()) } -/// The Solidity `address.transfer` and `address.send` call detection heuristic. -/// -/// # Why -/// This heuristic is an additional security feature to guard against re-entrancy attacks -/// in case contract authors violate Solidity best practices and use `address.transfer` or -/// `address.send`. -/// While contract authors are supposed to never use `address.transfer` or `address.send`, -/// for a small cost we can be extra defensive about it. -/// -/// # How -/// The gas stipend emitted by solc for `transfer` and `send` is not static, thus: -/// - Dynamically allow re-entrancy only for calls considered not transfer or send. -/// - Detected balance transfers will supply 0 deposit limit instead of `u256::MAX`. -/// -/// Calls are considered transfer or send if: -/// - (Input length | Output lenght) == 0; -/// - Gas <= 2300; -/// -/// # Returns -/// The call flags xlen `IntValue` and the deposit limit word `IntValue`. -fn call_reentrancy_heuristic<'ctx>( - context: &mut Context<'ctx>, +/// The runtime implements gas as `u64` so we clip the stipend to `u64::MAX`. +fn clip_call_gas<'ctx>( + context: &Context<'ctx>, gas: inkwell::values::IntValue<'ctx>, - input_length: inkwell::values::IntValue<'ctx>, - output_length: inkwell::values::IntValue<'ctx>, -) -> anyhow::Result<( - inkwell::values::IntValue<'ctx>, - inkwell::values::IntValue<'ctx>, -)> { - // Branch-free SSA implementation: First derive the heuristic boolean (int1) value. - let input_length_or_output_length = - context - .builder() - .build_or(input_length, output_length, "input_length_or_output_length")?; - let is_no_input_no_output = context.builder().build_int_compare( - inkwell::IntPredicate::EQ, - context.xlen_type().const_zero(), - input_length_or_output_length, - "is_no_input_no_output", - )?; - let gas_stipend = context - .word_type() - .const_int(SOLIDITY_TRANSFER_GAS_STIPEND_THRESHOLD, false); - let is_gas_stipend_for_transfer_or_send = context.builder().build_int_compare( - inkwell::IntPredicate::ULE, +) -> anyhow::Result> { + let builder = context.builder(); + + let clipped = context.register_type().const_all_ones(); + let is_overflow = builder.build_int_compare( + inkwell::IntPredicate::UGT, gas, - gas_stipend, - "is_gas_stipend_for_transfer_or_send", + builder.build_int_z_extend(clipped, context.word_type(), "gas_clipped")?, + "is_gas_overflow", )?; - let is_balance_transfer = context.builder().build_and( - is_no_input_no_output, - is_gas_stipend_for_transfer_or_send, - "is_balance_transfer", - )?; - let is_regular_call = context - .builder() - .build_not(is_balance_transfer, "is_balance_transfer_inverted")?; + let truncated = builder.build_int_truncate(gas, context.register_type(), "gas_truncated")?; + let call_gas = builder.build_select(is_overflow, clipped, truncated, "call_gas")?; - // Call flag: Left shift the heuristic boolean value. - let is_regular_call_xlen = context.builder().build_int_z_extend( - is_regular_call, - context.xlen_type(), - "is_balance_transfer_xlen", - )?; - let call_flags = context.builder().build_left_shift( - is_regular_call_xlen, - context.xlen_type().const_int(3, false), - "flags", - )?; - - // Deposit limit value: Sign-extended the heuristic boolean value. - let deposit_limit_value = context.builder().build_int_s_extend( - is_regular_call, - context.word_type(), - "deposit_limit_value", - )?; - - Ok((call_flags, deposit_limit_value)) + Ok(call_gas) } diff --git a/crates/runtime-api/src/polkavm_imports.c b/crates/runtime-api/src/polkavm_imports.c index 81a99b9..ec11516 100644 --- a/crates/runtime-api/src/polkavm_imports.c +++ b/crates/runtime-api/src/polkavm_imports.c @@ -53,7 +53,7 @@ POLKAVM_IMPORT(void, block_hash, uint32_t, uint32_t) POLKAVM_IMPORT(void, block_number, uint32_t) -POLKAVM_IMPORT(uint32_t, call, uint64_t, uint64_t, uint64_t, uint64_t, uint64_t, uint64_t) +POLKAVM_IMPORT(uint32_t, call_evm, uint32_t, uint32_t, uint32_t, uint64_t, uint64_t, uint64_t) POLKAVM_IMPORT(void, call_data_copy, uint32_t, uint32_t, uint32_t) @@ -71,7 +71,7 @@ POLKAVM_IMPORT(void, code_hash, uint32_t, uint32_t) POLKAVM_IMPORT(void, consume_all_gas) -POLKAVM_IMPORT(uint32_t, delegate_call, uint64_t, uint64_t, uint64_t, uint32_t, uint64_t, uint64_t) +POLKAVM_IMPORT(uint32_t, delegate_call_evm, uint32_t, uint32_t, uint64_t, uint64_t, uint64_t) POLKAVM_IMPORT(void, deposit_event, uint32_t, uint32_t, uint32_t, uint32_t) diff --git a/crates/runtime-api/src/polkavm_imports.rs b/crates/runtime-api/src/polkavm_imports.rs index 7158ed5..24e2058 100644 --- a/crates/runtime-api/src/polkavm_imports.rs +++ b/crates/runtime-api/src/polkavm_imports.rs @@ -16,7 +16,7 @@ pub static BLOCK_HASH: &str = "block_hash"; pub static BLOCK_NUMBER: &str = "block_number"; -pub static CALL: &str = "call"; +pub static CALL: &str = "call_evm"; pub static CALL_DATA_COPY: &str = "call_data_copy"; @@ -32,7 +32,7 @@ pub static CODE_SIZE: &str = "code_size"; pub static CODE_HASH: &str = "code_hash"; -pub static DELEGATE_CALL: &str = "delegate_call"; +pub static DELEGATE_CALL: &str = "delegate_call_evm"; pub static DEPOSIT_EVENT: &str = "deposit_event"; diff --git a/crates/solc-json-interface/src/standard_json/output/error/mod.rs b/crates/solc-json-interface/src/standard_json/output/error/mod.rs index 208e132..9787d18 100644 --- a/crates/solc-json-interface/src/standard_json/output/error/mod.rs +++ b/crates/solc-json-interface/src/standard_json/output/error/mod.rs @@ -113,10 +113,9 @@ impl Error { let message = r#" Warning: It looks like you are using '
.send/transfer()'. Using '
.send/transfer()' is deprecated and strongly discouraged! -The resolc compiler uses a heuristic to detect '
.send/transfer()' calls, -which disables call re-entrancy and supplies all remaining gas instead of the 2300 gas stipend. -However, detection is not guaranteed. You are advised to carefully test this, employ -re-entrancy guards or use the withdrawal pattern instead! +The revive runtime uses a heuristic to detect '
.send/transfer()' calls and +the gas stipend used by the runtime is different from the EVM. +You are advised to carefully test this, employ re-entrancy guards or use the withdrawal pattern instead! Learn more on https://docs.soliditylang.org/en/latest/security-considerations.html#reentrancy and https://docs.soliditylang.org/en/latest/common-patterns.html#withdrawal-from-contracts "#