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
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
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,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"] }
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 = { 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
Loading