llvm-context: disable call re-entrancy for send and transfer (#196)

This commit is contained in:
xermicus
2025-02-06 16:49:50 +01:00
committed by GitHub
parent 10b8ff989c
commit 60fc09f787
8 changed files with 354 additions and 35 deletions
+2
View File
@@ -15,6 +15,8 @@ Supported `polkadot-sdk` rev: `274a781e8ca1a9432c7ec87593bd93214abbff50`
- Removed support for legacy EVM assembly (EVMLA) translation.
- integration: identify cached code blobs on source code to fix potential confusions.
- Setting base, include or allow paths in emscripten is now a hard error.
- Employ a heuristic to detect `address.transfer` and `address.send` calls.
If detected, the re-entrant call flag is not set and 0 deposit limit is endowed.
### Fixed
- Solidity: Add the solc `--libraries` files to sources.
+81
View File
@@ -0,0 +1,81 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8;
/* runner.json
{
"differential": true,
"actions": [
{
"Upload": {
"code": {
"Solidity": {
"contract": "BalanceReceiver"
}
}
}
},
{
"Instantiate": {
"code": {
"Solidity": {
"contract": "Balance"
}
},
"value": 24
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a"
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "6ada15d90000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a"
}
}
]
}
*/
contract BalanceReceiver {
constructor() payable {}
fallback() external payable {}
}
contract Balance {
constructor() payable {
// 0 to EOA
transfer_to(payable(address(0xdeadbeef)), 0);
send_to(payable(address(0xdeadbeef)), 0);
// 1 to EOA
transfer_to(payable(address(0xcafebabe)), 1);
send_to(payable(address(0xcafebabe)), 1);
BalanceReceiver balanceReceiver = new BalanceReceiver();
// 0 to contract
transfer_to(payable(address(balanceReceiver)), 0);
send_to(payable(address(balanceReceiver)), 0);
// 1 to contract
transfer_to(payable(address(balanceReceiver)), 1);
send_to(payable(address(balanceReceiver)), 1);
}
function transfer_to(address payable _dest, uint _amount) public payable {
_dest.transfer(_amount);
}
function send_to(address payable _dest, uint _amount) public payable {
require(_dest.send(_amount));
}
}
+75
View File
@@ -0,0 +1,75 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8;
/* runner.json
{
"differential": false,
"actions": [
{
"Instantiate": {
"code": {
"Solidity": {
"contract": "Send"
}
},
"value": 211
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a"
}
},
{
"VerifyCall": {
"success": true
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000001"
}
},
{
"VerifyCall": {
"success": false
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000000"
}
},
{
"VerifyCall": {
"success": false
}
}
]
}
*/
contract Send {
constructor() payable {}
function transfer_self(uint _amount) public payable {
transfer_to(payable(address(this)), _amount);
}
function transfer_to(address payable _dest, uint _amount) public payable {
if (_dest.send(_amount)) {}
}
fallback() external {}
receive() external payable {}
}
+32 -11
View File
@@ -3,7 +3,7 @@ pragma solidity ^0.8;
/* runner.json
{
"differential": true,
"differential": false,
"actions": [
{
"Instantiate": {
@@ -12,7 +12,7 @@ pragma solidity ^0.8;
"contract": "Transfer"
}
},
"value": 11
"value": 211
}
},
{
@@ -23,12 +23,35 @@ pragma solidity ^0.8;
"data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a"
}
},
{
"VerifyCall": {
"success": true
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "fb9e8d0500000000000000000000000003030303030303030303030303030303030303030000000000000000000000000000000000000000000000000000000000000001"
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000001"
}
},
{
"VerifyCall": {
"success": false
}
},
{
"Call": {
"dest": {
"Instantiated": 0
},
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000000"
}
},
{
"VerifyCall": {
"success": false
}
}
]
@@ -36,19 +59,17 @@ pragma solidity ^0.8;
*/
contract Transfer {
constructor() payable {
transfer_self(msg.value);
}
function address_self() internal view returns (address payable) {
return payable(address(this));
}
constructor() payable {}
function transfer_self(uint _amount) public payable {
transfer_to(address_self(), _amount);
transfer_to(payable(address(this)), _amount);
}
function transfer_to(address payable _dest, uint _amount) public payable {
_dest.transfer(_amount);
}
fallback() external {}
receive() external payable {}
}
+14
View File
@@ -156,6 +156,20 @@ case!("DivisionArithmetics.sol", DivisionArithmetics, sdivCall, division_arithme
case!("DivisionArithmetics.sol", DivisionArithmetics, modCall, division_arithmetics_mod, n: U256, d: U256);
case!("DivisionArithmetics.sol", DivisionArithmetics, smodCall, division_arithmetics_smod, n: I256, d: I256);
sol!(
contract Send {
function transfer_self(uint _amount) public payable;
}
);
case!("Send.sol", Send, transfer_selfCall, send_self, amount: U256);
sol!(
contract Transfer {
function transfer_self(uint _amount) public payable;
}
);
case!("Transfer.sol", Transfer, transfer_selfCall, transfer_self, amount: U256);
sol!(
contract MStore8 {
function mStore8(uint value) public pure returns (uint256 word);
+48 -2
View File
@@ -37,10 +37,10 @@ test_spec!(events, "Events", "Events.sol");
test_spec!(storage, "Storage", "Storage.sol");
test_spec!(mstore8, "MStore8", "MStore8.sol");
test_spec!(address, "Context", "Context.sol");
test_spec!(balance, "Value", "Value.sol");
test_spec!(value, "Value", "Value.sol");
test_spec!(create, "CreateB", "Create.sol");
test_spec!(call, "Caller", "Call.sol");
test_spec!(transfer, "Transfer", "Transfer.sol");
test_spec!(balance, "Balance", "Balance.sol");
test_spec!(return_data_oob, "ReturnDataOob", "ReturnDataOob.sol");
test_spec!(immutables, "Immutables", "Immutables.sol");
test_spec!(transaction, "Transaction", "Transaction.sol");
@@ -52,6 +52,8 @@ test_spec!(gas_limit, "GasLimit", "GasLimit.sol");
test_spec!(base_fee, "BaseFee", "BaseFee.sol");
test_spec!(coinbase, "Coinbase", "Coinbase.sol");
test_spec!(create2, "CreateB", "Create2.sol");
test_spec!(transfer, "Transfer", "Transfer.sol");
test_spec!(send, "Send", "Send.sol");
fn instantiate(path: &str, contract: &str) -> Vec<SpecsAction> {
vec![Instantiate {
@@ -434,3 +436,47 @@ 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();
}
+95 -18
View File
@@ -8,6 +8,7 @@ use crate::polkavm::Dependency;
const STATIC_CALL_FLAG: u32 = 0b0001_0000;
const REENTRANT_CALL_FLAG: u32 = 0b0000_1000;
const SOLIDITY_TRANSFER_GAS_STIPEND_THRESHOLD: u64 = 2300;
/// Translates a contract call.
#[allow(clippy::too_many_arguments)]
@@ -37,26 +38,24 @@ where
let output_offset = context.safe_truncate_int_to_xlen(output_offset)?;
let output_length = context.safe_truncate_int_to_xlen(output_length)?;
// TODO: What to supply here? Is there a weight to gas?
let _gas = context
.builder()
.build_int_truncate(gas, context.integer_type(64), "gas")?;
let input_pointer = context.build_heap_gep(input_offset, input_length)?;
let output_pointer = context.build_heap_gep(output_offset, output_length)?;
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 = if static_call {
REENTRANT_CALL_FLAG | STATIC_CALL_FLAG
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(),
)
} else {
REENTRANT_CALL_FLAG
call_reentrancy_heuristic(context, gas, input_length, output_length)?
};
let flags = context.xlen_type().const_int(flags as u64, false);
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(),
@@ -119,7 +118,7 @@ where
#[allow(clippy::too_many_arguments)]
pub fn delegate_call<'ctx, D>(
context: &mut Context<'ctx, D>,
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>,
@@ -137,11 +136,6 @@ where
let output_offset = context.safe_truncate_int_to_xlen(output_offset)?;
let output_length = context.safe_truncate_int_to_xlen(output_length)?;
// TODO: What to supply here? Is there a weight to gas?
let _gas = context
.builder()
.build_int_truncate(gas, context.integer_type(64), "gas")?;
let input_pointer = context.build_heap_gep(input_offset, input_length)?;
let output_pointer = context.build_heap_gep(output_offset, output_length)?;
@@ -221,3 +215,86 @@ where
.resolve_library(path.as_str())?
.as_basic_value_enum())
}
/// 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, D>(
context: &mut Context<'ctx, D>,
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>,
)>
where
D: Dependency + Clone,
{
// 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,
gas,
gas_stipend,
"is_gas_stipend_for_transfer_or_send",
)?;
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")?;
// 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))
}
@@ -54,11 +54,14 @@ implement other signature schemes.
/// Returns the `<address payable>`'s `send` and `transfer` methods usage error.
pub fn message_send_and_transfer(src: Option<&str>) -> Self {
let message = r#"
Warning: It looks like you are using '<address payable>.send/transfer(<X>)'. Such balance
transfer calls will supply all remaining gas and disable call re-entrancy instead of
supplying the 2300 gas stipend. However, the compiler uses a heuristic to detect the expected
2300 gas stipend. You are advised to carefully test this to ensure the desired behavior.
Warning: It looks like you are using '<address payable>.send/transfer(<X>)'.
Using '<address payable>.send/transfer(<X>)' is deprecated and strongly discouraged!
The resolc compiler uses a heuristic to detect '<address payable>.send/transfer(<X>)' 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!
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
"#
.to_owned();