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

fix: string slice panic & automated smart contract tests #76

Merged
merged 10 commits into from
Apr 17, 2024
5 changes: 4 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
rust: nightly
- name: "Tests"
cmd: nextest
args: run --workspace --all-features --features cacao --retries 3
args: run --workspace --all-features
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--all-features implies --features cacao

--retries doesn't seem to be necessary in latest version

rust: stable
- name: "Documentation Tests"
cmd: test
Expand All @@ -54,6 +54,9 @@ jobs:
profile: default
override: true

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1

- uses: Swatinem/rust-cache@v2

- uses: taiki-e/install-action@v1
Expand Down
2 changes: 1 addition & 1 deletion blockchain_api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "0.1.0"
edition = "2021"

[dependencies]
relay_rpc = { path = "../relay_rpc" }
relay_rpc = { path = "../relay_rpc", features = ["cacao"] }
Copy link
Member Author

@chris13524 chris13524 Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually compile without this; forgot in previous PRs. Wasn't caught because CI here always sets this feature.

reqwest = "0.11"
serde = "1.0"
tokio = { version = "1.0", features = ["test-util", "macros"] }
Expand Down
2 changes: 2 additions & 0 deletions relay_rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ cacao = [
"dep:alloy-json-abi",
"dep:alloy-sol-types",
"dep:alloy-primitives",
"dep:alloy-node-bindings"
]

[dependencies]
Expand Down Expand Up @@ -49,6 +50,7 @@ alloy-transport = { git = "https://github.com/alloy-rs/alloy.git", rev = "e6f98e
alloy-transport-http = { git = "https://github.com/alloy-rs/alloy.git", rev = "e6f98e1", optional = true }
alloy-rpc-types = { git = "https://github.com/alloy-rs/alloy.git", rev = "e6f98e1", optional = true }
alloy-json-rpc = { git = "https://github.com/alloy-rs/alloy.git", rev = "e6f98e1", optional = true }
alloy-node-bindings = { git = "https://github.com/alloy-rs/alloy.git", rev = "e6f98e1", optional = true }
alloy-json-abi = { version = "0.6.2", optional = true }
alloy-sol-types = { version = "0.6.2", optional = true }
alloy-primitives = { version = "0.6.2", optional = true }
Expand Down
81 changes: 81 additions & 0 deletions relay_rpc/src/auth/cacao/signature/eip1271/TestContract.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
pragma solidity ^0.8.25;

// https://eips.ethereum.org/EIPS/eip-1271#reference-implementation

contract TestContract {
address owner;

constructor() {
owner = msg.sender;
}

/**
* @notice Verifies that the signer is the owner of the signing contract.
*/
function isValidSignature(
bytes32 _hash,
bytes calldata _signature
) external view returns (bytes4) {
// Validate signatures
if (recoverSigner(_hash, _signature) == owner) {
return 0x1626ba7e;
} else {
return 0xffffffff;
}
}

/**
* @notice Recover the signer of hash, assuming it's an EOA account
* @dev Only for EthSign signatures
* @param _hash Hash of message that was signed
* @param _signature Signature encoded as (bytes32 r, bytes32 s, uint8 v)
*/
function recoverSigner(
bytes32 _hash,
bytes memory _signature
) internal pure returns (address signer) {
require(_signature.length == 65, "SignatureValidator#recoverSigner: invalid signature length");

// Variables are not scoped in Solidity.
uint8 v = uint8(_signature[64]);
bytes32 r;
bytes32 s;
assembly {
// Slice the signature into r and s components
r := mload(add(_signature, 32))
s := mload(add(_signature, 64))
}

// EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature
// unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines
// the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1, and for v in (282): v ∈ {27, 28}. Most
// signatures from current libraries generate a unique signature with an s-value in the lower half order.
//
// If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value
// with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or
// vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept
// these malleable signatures as well.
//
// Source OpenZeppelin
// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/cryptography/ECDSA.sol

if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
revert("SignatureValidator#recoverSigner: invalid signature 's' value");
}

if (v != 27 && v != 28) {
revert("SignatureValidator#recoverSigner: invalid signature 'v' value");
}

// Recover ECDSA signer
signer = ecrecover(_hash, v, r, s);

// Prevent signer from being 0x0
require(
signer != address(0x0),
"SignatureValidator#recoverSigner: INVALID_SIGNER"
);

return signer;
}
}
133 changes: 130 additions & 3 deletions relay_rpc/src/auth/cacao/signature/eip1271/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,13 @@ pub async fn verify_eip1271(
}
})?;

if result[..4] == MAGIC_VALUE.to_be_bytes().to_vec() {
Ok(true)
let magic = result.get(..4);
if let Some(magic) = magic {
if magic == MAGIC_VALUE.to_be_bytes().to_vec() {
Ok(true)
} else {
Err(CacaoError::Verification)
}
} else {
Err(CacaoError::Verification)
}
Expand All @@ -68,15 +73,20 @@ mod test {
use {
super::*,
crate::auth::cacao::signature::{eip191::eip191_bytes, strip_hex_prefix},
alloy_node_bindings::{Anvil, AnvilInstance},
alloy_primitives::address,
k256::{ecdsa::SigningKey, elliptic_curve::SecretKey, Secp256k1},
regex::Regex,
sha3::{Digest, Keccak256},
std::process::Stdio,
tokio::process::Command,
};

// Manual test. Paste address, signature, message, and project ID to verify
// function
#[tokio::test]
#[ignore]
async fn test_eip1271() {
async fn test_eip1271_manual() {
let address = address!("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
let signature = "xxx";
let signature = data_encoding::HEXLOWER_PERMISSIVE
Expand All @@ -94,4 +104,121 @@ mod test {
.await
.unwrap());
}

async fn spawn_anvil() -> (AnvilInstance, Url, SecretKey<Secp256k1>) {
let anvil = Anvil::new().spawn();
let provider = anvil.endpoint().parse().unwrap();
let private_key = anvil.keys().first().unwrap().clone();
(anvil, provider, private_key)
}

async fn deploy_contract(rpc_url: &Url, private_key: &SecretKey<Secp256k1>) -> Address {
let key_encoded = data_encoding::HEXLOWER_PERMISSIVE.encode(&private_key.to_bytes());
let output = Command::new("forge")
.args([
"create",
"--contracts",
"relay_rpc/src/auth/cacao/signature/eip1271",
"TestContract",
"--rpc-url",
rpc_url.as_str(),
"--private-key",
&key_encoded,
"--cache-path",
"target/.forge/cache",
"--out",
"target/.forge/out",
])
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.unwrap()
.wait_with_output()
.await
.unwrap();
let output = String::from_utf8(output.stdout).unwrap();
let (_, [contract_address]) = Regex::new("Deployed to: (0x[0-9a-fA-F]+)")
.unwrap()
.captures(&output)
.unwrap()
.extract();
contract_address.parse().unwrap()
}

fn sign_message(message: &str, private_key: &SecretKey<Secp256k1>) -> Vec<u8> {
let (signature, recovery): (k256::ecdsa::Signature, _) =
SigningKey::from_bytes(&private_key.to_bytes())
.unwrap()
.sign_digest_recoverable(Keccak256::new_with_prefix(eip191_bytes(message)))
.unwrap();
let signature = signature.to_bytes();
// need for +27 is mentioned in EIP-1271 reference implementation
[&signature[..], &[recovery.to_byte() + 27]].concat()
}

fn message_hash(message: &str) -> [u8; 32] {
Keccak256::new_with_prefix(eip191_bytes(message)).finalize()[..]
.try_into()
.unwrap()
}

#[tokio::test]
async fn test_eip1271_pass() {
let (_anvil, rpc_url, private_key) = spawn_anvil().await;
let contract_address = deploy_contract(&rpc_url, &private_key).await;

let message = "xxx";
let signature = sign_message(message, &private_key);

assert!(
verify_eip1271(signature, contract_address, &message_hash(message), rpc_url)
.await
.unwrap()
);
}

#[tokio::test]
async fn test_eip1271_fail() {
let (_anvil, rpc_url, private_key) = spawn_anvil().await;
let contract_address = deploy_contract(&rpc_url, &private_key).await;

let message = "xxx";
let mut signature = sign_message(message, &private_key);
signature[0] = signature[0].wrapping_add(1);

assert!(matches!(
verify_eip1271(signature, contract_address, &message_hash(message), rpc_url).await,
Err(CacaoError::Verification)
));
}

#[tokio::test]
async fn test_eip1271_fail_wrong_signer() {
let (anvil, rpc_url, private_key) = spawn_anvil().await;
let contract_address = deploy_contract(&rpc_url, &private_key).await;

let message = "xxx";
let signature = sign_message(message, &anvil.keys()[1]);

assert!(matches!(
verify_eip1271(signature, contract_address, &message_hash(message), rpc_url).await,
Err(CacaoError::Verification)
));
}

#[tokio::test]
async fn test_eip1271_fail_wrong_contract_address() {
let (_anvil, rpc_url, private_key) = spawn_anvil().await;
let mut contract_address = deploy_contract(&rpc_url, &private_key).await;

contract_address.0[0] = contract_address.0[0].wrapping_add(1);

let message = "xxx";
let signature = sign_message(message, &private_key);

assert!(matches!(
verify_eip1271(signature, contract_address, &message_hash(message), rpc_url).await,
Err(CacaoError::Verification)
));
}
}
Loading