Skip to content

Commit

Permalink
EVM: Allow Null Addresses to convert into f4 Addresses (#1003)
Browse files Browse the repository at this point in the history
* account for null and native precompile address in eth to f4 address conversions

* fmt & err message fix

* be explicit about rejecting null address in EAM

* add eam test for null address
  • Loading branch information
mriise authored Jan 9, 2023
1 parent 93d940b commit 56339c2
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 8 deletions.
24 changes: 22 additions & 2 deletions actors/eam/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl EthAddress {
#[inline]
fn is_precompile(&self) -> bool {
// Exact index is not checked since it is unknown to the EAM what precompiles exist in the EVM actor.
// 0 indexes of both ranges are not assignable as well but are _not_ precompile address.
let [prefix, middle @ .., _index] = self.0;
(prefix == 0xfe || prefix == 0x00) && middle == [0u8; 18]

This comment has been minimized.

Copy link
@wadealexc

wadealexc Jan 10, 2023

is_precompile doesn't actually check that the index is > 0, meaning it currently returns true for the zero address. Is that intended? The comments and changes here imply it's not.

}
Expand All @@ -73,10 +74,15 @@ impl EthAddress {
self.0[0] == 0xff && self.0[1..12].iter().all(|&i| i == 0)
}

#[inline]
fn is_null(&self) -> bool {
self.0 == [0; 20]
}

/// Returns true if the EthAddress is "reserved" (cannot be assigned by the EAM).
#[inline]
fn is_reserved(&self) -> bool {
self.is_precompile() || self.is_id()
self.is_precompile() || self.is_id() || self.is_null()
}
}

Expand Down Expand Up @@ -267,13 +273,27 @@ mod test {
create_actor(&mut rt, creator, new_addr, Vec::new()).unwrap_err().exit_code()
);

// Reject Precompile.
// Reject EVM Precompile.
let mut new_addr = EthAddress([0; 20]);
new_addr.0[19] = 0x20;
assert_eq!(
ExitCode::USR_FORBIDDEN,
create_actor(&mut rt, creator, new_addr, Vec::new()).unwrap_err().exit_code()
);

// Reject Native Precompile.
new_addr.0[0] = 0xfe;
assert_eq!(
ExitCode::USR_FORBIDDEN,
create_actor(&mut rt, creator, new_addr, Vec::new()).unwrap_err().exit_code()
);

// Reject Null.
let new_addr = EthAddress([0; 20]);
assert_eq!(
ExitCode::USR_FORBIDDEN,
create_actor(&mut rt, creator, new_addr, Vec::new()).unwrap_err().exit_code()
);
}

#[test]
Expand Down
33 changes: 30 additions & 3 deletions actors/evm/src/interpreter/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use fvm_ipld_encoding::{serde, strict_bytes};
use fvm_shared::address::Address;
use fvm_shared::ActorID;

use super::precompiles::is_reserved_precompile_address;

/// A Filecoin address as represented in the FEVM runtime (also called EVM-form).
///
/// TODO this type will eventually handle f4 address detection.
Expand Down Expand Up @@ -38,10 +40,10 @@ impl TryFrom<EthAddress> for Address {
impl TryFrom<&EthAddress> for Address {
type Error = StatusCode;
fn try_from(addr: &EthAddress) -> Result<Self, Self::Error> {
if addr.0[..19] == [0; 19] {
if is_reserved_precompile_address(addr.0) {
return Err(StatusCode::BadAddress(format!(
"cannot convert precompile {} to an f4 address",
addr.0[19]
"Cannot convert a precompile address: {:?} to an f4 address",
addr
)));
}

Expand Down Expand Up @@ -93,6 +95,8 @@ impl AsRef<[u8]> for EthAddress {

#[cfg(test)]
mod tests {
use fvm_shared::address::Address;

use crate::interpreter::address::EthAddress;
use crate::U256;

Expand Down Expand Up @@ -158,4 +162,27 @@ mod tests {
vec![0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01].as_slice() // ID address (u64 big endian) (8 bytes)
] => None,
}

#[test]
#[allow(unused)]
fn precompile_reserved_conversion() {
// in range precompile addresses
let addr = EthAddress(hex_literal::hex!("fe00000000000000000000000000000000000001"));
Address::try_from(addr).expect_err("can't convert precompile into f4!");
let addr = EthAddress(hex_literal::hex!("0000000000000000000000000000000000000001"));
Address::try_from(addr).expect_err("can't convert precompile into f4!");

// can convert null address
let addr = EthAddress(hex_literal::hex!("0000000000000000000000000000000000000000"));
let _: Address = addr.try_into().unwrap();
// can convert 0 index native prefix
let addr = EthAddress(hex_literal::hex!("fe00000000000000000000000000000000000000"));
let _: Address = addr.try_into().unwrap();

// out of range, but reserved
let addr = EthAddress(hex_literal::hex!("fe000000000000000000000000000000000000aa"));
Address::try_from(addr).expect_err("can't convert precompile into f4!");
let addr = EthAddress(hex_literal::hex!("00000000000000000000000000000000000000aa"));
Address::try_from(addr).expect_err("can't convert precompile into f4!");
}
}
22 changes: 19 additions & 3 deletions actors/evm/src/interpreter/precompiles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ const fn gen_native_precompiles<RT: Runtime>() -> [PrecompileFn<RT>; 4] {
}
}

pub fn is_reserved_precompile_address(addr: [u8; 20]) -> bool {
let [prefix, middle @ .., index] = addr;
(prefix == 0x00 || prefix == NATIVE_PRECOMPILE_ADDRESS_PREFIX)
&& middle == [0u8; 18]
&& index > 0
}

pub struct Precompiles<RT>(PhantomData<RT>);

impl<RT: Runtime> Precompiles<RT> {
Expand All @@ -70,8 +77,8 @@ impl<RT: Runtime> Precompiles<RT> {
fn lookup_precompile(addr: &[u8; 32]) -> Option<PrecompileFn<RT>> {
// unrwap will never panic, 32 - 12 = 20
let addr: [u8; 20] = addr[12..].try_into().unwrap();
let [prefix, middle @ .., index] = addr;
if middle == [0u8; 18] && index > 0 {
let [prefix, _m @ .., index] = addr;
if is_reserved_precompile_address(addr) {
let index = index as usize - 1;
match prefix {
NATIVE_PRECOMPILE_ADDRESS_PREFIX => Self::NATIVE_PRECOMPILES.get(index),
Expand Down Expand Up @@ -146,26 +153,29 @@ impl NativeType {
mod test {
use fil_actors_runtime::test_utils::MockRuntime;

use crate::interpreter::address::EthAddress;
use crate::interpreter::{address::EthAddress, precompiles::is_reserved_precompile_address};

use super::Precompiles;

#[test]
fn is_native_precompile() {
let addr = EthAddress(hex_literal::hex!("fe00000000000000000000000000000000000001"));
assert!(Precompiles::<MockRuntime>::is_precompile(&addr.as_evm_word()));
assert!(is_reserved_precompile_address(addr.0));
}

#[test]
fn is_evm_precompile() {
let addr = EthAddress(hex_literal::hex!("0000000000000000000000000000000000000001"));
assert!(Precompiles::<MockRuntime>::is_precompile(&addr.as_evm_word()));
assert!(is_reserved_precompile_address(addr.0));
}

#[test]
fn is_over_precompile() {
let addr = EthAddress(hex_literal::hex!("ff00000000000000000000000000000000000001"));
assert!(!Precompiles::<MockRuntime>::is_precompile(&addr.as_evm_word()));
assert!(!is_reserved_precompile_address(addr.0));
}

#[test]
Expand All @@ -174,12 +184,15 @@ mod test {
let native_addr = EthAddress(hex_literal::hex!("0000000000000000000000000000000000000000"));
assert!(!Precompiles::<MockRuntime>::is_precompile(&eth_addr.as_evm_word()));
assert!(!Precompiles::<MockRuntime>::is_precompile(&native_addr.as_evm_word()));
assert!(!is_reserved_precompile_address(eth_addr.0));
assert!(!is_reserved_precompile_address(native_addr.0));
}

#[test]
fn between_precompile() {
let addr = EthAddress(hex_literal::hex!("a000000000000000000000000000000000000001"));
assert!(!Precompiles::<MockRuntime>::is_precompile(&addr.as_evm_word()));
assert!(!is_reserved_precompile_address(addr.0));
}

#[test]
Expand All @@ -188,5 +201,8 @@ mod test {
let native_addr = EthAddress(hex_literal::hex!("0000000000000000000000000000000000000020"));
assert!(!Precompiles::<MockRuntime>::is_precompile(&eth_addr.as_evm_word()));
assert!(!Precompiles::<MockRuntime>::is_precompile(&native_addr.as_evm_word()));
// reserved doesn't check index is within range
assert!(is_reserved_precompile_address(eth_addr.0));
assert!(is_reserved_precompile_address(native_addr.0));
}
}

0 comments on commit 56339c2

Please sign in to comment.