Skip to content

Commit

Permalink
fix: string slice panic & automated smart contract tests (#76)
Browse files Browse the repository at this point in the history
  • Loading branch information
chris13524 authored Apr 17, 2024
1 parent b4c8f4c commit 2ce3a45
Show file tree
Hide file tree
Showing 13 changed files with 385 additions and 21 deletions.
9 changes: 8 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
rust: stable
- name: "Documentation Tests"
cmd: test
Expand All @@ -54,6 +54,13 @@ jobs:
profile: default
override: true

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

# pre-build contracts to avoid race condition installing solc during `forge create` in tests
- name: Build contracts
run: forge build -C contracts --cache-path=target/.forge/cache --out=target/.forge/out

- uses: Swatinem/rust-cache@v2

- uses: taiki-e/install-action@v1
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,6 @@ required-features = ["client", "rpc"]
[[example]]
name = "webhook"
required-features = ["client", "rpc"]

[lints.clippy]
indexing_slicing = "deny"
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ The core Relay client. Provides access to all available Relay RPC methods to bui

Provides all of the Relay domain types (e.g. `ClientId`, `ProjectId` etc.) as well as auth token generation and validation functionality.

### Test dependencies

Foundry is required to be installed to your system for testing: <https://book.getfoundry.sh/getting-started/installation>

# License

[Apache License (Version 2.0)](LICENSE)
5 changes: 4 additions & 1 deletion blockchain_api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ version = "0.1.0"
edition = "2021"

[dependencies]
relay_rpc = { path = "../relay_rpc" }
relay_rpc = { path = "../relay_rpc", features = ["cacao"] }
reqwest = { version = "0.12.2", features = ["json"] }
serde = "1.0"
tokio = { version = "1.0", features = ["test-util", "macros"] }
tracing = "0.1.40"
url = "2"

[lints.clippy]
indexing_slicing = "deny"
81 changes: 81 additions & 0 deletions contracts/Eip1271Mock.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 Eip1271Mock {
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;
}
}
3 changes: 3 additions & 0 deletions relay_client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ tokio-tungstenite = "0.21.0"
futures-channel = "0.3"
tokio-stream = "0.1"
tokio-util = "0.7"

[lints.clippy]
indexing_slicing = "deny"
5 changes: 5 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 @@ -48,10 +49,14 @@ 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 }
strum = { version = "0.26", features = ["strum_macros", "derive"] }

[dev-dependencies]
tokio = { version = "1.35.1", features = ["test-util", "macros"] }

[lints.clippy]
indexing_slicing = "deny"
4 changes: 4 additions & 0 deletions relay_rpc/src/auth/cacao.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use {
payload::Payload,
signature::{eip1271::get_rpc_url::GetRpcUrl, Signature},
},
alloy_primitives::hex::FromHexError,
core::fmt::Debug,
serde::{Deserialize, Serialize},
serde_json::value::RawValue,
Expand Down Expand Up @@ -32,6 +33,9 @@ pub enum CacaoError {
#[error("Invalid address")]
AddressInvalid,

#[error("Address not EIP-191")]
AddressNotEip191(FromHexError),

#[error("EIP-1271 signatures not supported")]
Eip1271NotSupported,

Expand Down
103 changes: 99 additions & 4 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 @@ -67,16 +72,21 @@ pub async fn verify_eip1271(
mod test {
use {
super::*,
crate::auth::cacao::signature::{eip191::eip191_bytes, strip_hex_prefix},
crate::auth::cacao::signature::{
eip191::eip191_bytes,
strip_hex_prefix,
test_helpers::{deploy_contract, message_hash, sign_message, spawn_anvil},
},
alloy_primitives::address,
k256::ecdsa::SigningKey,
sha3::{Digest, Keccak256},
};

// 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,89 @@ mod test {
.await
.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_wrong_signature() {
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.first_mut().unwrap() = signature.first().unwrap().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,
&SigningKey::from_bytes(&anvil.keys().get(1).unwrap().to_bytes()).unwrap(),
);

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.first_mut().unwrap() =
contract_address.0.first().unwrap().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)
));
}

#[tokio::test]
async fn test_eip1271_wrong_message() {
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);

let message2 = "yyy";
assert!(matches!(
verify_eip1271(
signature,
contract_address,
&message_hash(message2),
rpc_url
)
.await,
Err(CacaoError::Verification)
));
}
}
Loading

0 comments on commit 2ce3a45

Please sign in to comment.