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.

Sign up for GitHub

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: enforce ERC-55 addresses #368

Merged
merged 3 commits into from
Feb 19, 2024
Merged

fix: enforce ERC-55 addresses #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81b0259
fix: enforce ERC-55
chris13524 Feb 15, 2024
d07dee5
chore: avoid panic
chris13524 Feb 16, 2024
1658283
chore: prevent introduction of conditionally matching not going undet…
chris13524 Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 112 additions & 75 deletions src/model/types/account_id.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use {
sha2::Digest,
sha3::Keccak256,
std::sync::Arc,
tracing::info,
thiserror::Error,
};

#[derive(
Expand All @@ -32,9 +32,14 @@ impl AccountId {
}
}

#[derive(Debug, thiserror::Error)]
#[error("Account ID is is not a valid CAIP-10 account ID or uses an unsupported namespace")]
pub struct AccountIdError;
#[derive(Debug, PartialEq, Eq, Error)]
pub enum AccountIdError {
#[error("Account ID is is not a valid CAIP-10 account ID or uses an unsupported namespace")]
Namespace,

#[error("Account ID appears to be eip155 but: {0}")]
Eip155(#[from] Eip155Error),
}

impl TryFrom<String> for AccountId {
type Error = AccountIdError;
Expand All @@ -48,15 +53,11 @@ impl TryFrom<&str> for AccountId {
type Error = AccountIdError;

fn try_from(s: &str) -> Result<Self, Self::Error> {
if is_eip155_account(s) {
let erc55 = ensure_erc_55(s);
info!(
"Address is {}ERC-55 compliant",
if erc55 == s { "" } else { "not " }
);
Ok(Self(Arc::from(erc55)))
if s.starts_with("eip155:") {
is_valid_eip155_account(s)?;
Ok(Self(Arc::from(s)))
} else {
Err(AccountIdError)
Err(AccountIdError::Namespace)
}
}
}
Expand All @@ -74,11 +75,31 @@ impl<'a> Deserialize<'a> for AccountId {
}
}

fn is_eip155_account(account_id: &str) -> bool {
#[derive(Debug, PartialEq, Eq, Error)]
pub enum Eip155Error {
#[error("does not match regex")]
Regex,

#[error("does not pass ERC-55 checksum")]
Erc55,
}

fn is_valid_eip155_account(account_id: &str) -> Result<(), Eip155Error> {
static PATTERN_CELL: OnceCell<regex::Regex> = OnceCell::new();
let pattern =
PATTERN_CELL.get_or_init(|| regex::Regex::new(r"^eip155:\d+:0x[0-9a-fA-F]{40}$").unwrap());
pattern.is_match(account_id)
let pattern = PATTERN_CELL
.get_or_init(|| regex::Regex::new(r"^eip155:\d+:0x([0-9a-fA-F]{40})$").unwrap());

if let Some(caps) = pattern.captures(account_id) {
let (_, [address]) = &caps.extract();
let erc55 = erc_55_checksum_encode(&address.to_ascii_lowercase()).collect::<String>();
if &erc55 == address {
Ok(())
} else {
Err(Eip155Error::Erc55)
}
} else {
Err(Eip155Error::Regex)
}
}

#[derive(Debug, thiserror::Error)]
Expand All @@ -105,26 +126,9 @@ impl AccountId {
}
}

fn ensure_erc_55(s: &str) -> String {
if s.starts_with("eip155:") {
let ox = "0x";
if let Some(ox_start) = s.find(ox) {
let hex_start = ox_start + ox.len();
s[0..hex_start]
.chars()
.chain(erc_55_checksum_encode(&s[hex_start..].to_ascii_lowercase()))
.collect()
} else {
// If no 0x then address is very invalid anyway. Not validating this for now, goal is just to avoid duplicates.
s.to_owned()
}
} else {
s.to_owned()
}
}

// Encodes a lowercase hex address with ERC-55 checksum
fn erc_55_checksum_encode(s: &str) -> impl Iterator<Item = char> + '_ {
// Encodes a lowercase hex address without '0x' with ERC-55 checksum
// If a non-lowercase hex value, or a non-address is passed, the behavior is undefined
pub fn erc_55_checksum_encode(s: &str) -> impl Iterator<Item = char> + '_ {
let address_hash = hex::encode(Keccak256::default().chain_update(s).finalize());
s.chars().enumerate().map(move |(i, c)| {
if !c.is_numeric() && address_hash.as_bytes()[i] > b'7' {
Expand All @@ -145,23 +149,23 @@ mod test {

// Ethereum mainnet (valid/checksummed)
let test = "eip155:1:0x22227A31dd842196A246d8f3b775998560eAa61d";
assert_eq!(test, ensure_erc_55(test));
assert!(is_valid_eip155_account(test).is_ok());

// Ethereum mainnet (will not validate in EIP155-conformant systems)
let test = "eip155:1:0x22227a31dd842196a246d8f3b775998560eaa61d";
assert_ne!(test, ensure_erc_55(test));
assert!(is_valid_eip155_account(test).is_err());

// Polygon mainnet (valid/checksummed)
let test = "eip155:137:0x0495766cD136138Fc492Dd499B8DC87A92D6685b";
assert_eq!(test, ensure_erc_55(test));
assert!(is_valid_eip155_account(test).is_ok());

// Polygon mainnet (will not validate in EIP155-conformant systems)
let test = "eip155:137:0x0495766CD136138FC492DD499B8DC87A92D6685B";
assert_ne!(test, ensure_erc_55(test));
assert!(is_valid_eip155_account(test).is_err());

// Not EIP155
let junk = "jkF53jF";
assert_eq!(junk, ensure_erc_55(junk));
assert!(is_valid_eip155_account(junk).is_err());
}

#[test]
Expand Down Expand Up @@ -201,39 +205,72 @@ mod test {
}

#[test]
fn test_is_eip155_account() {
assert!(is_eip155_account(
"eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(is_eip155_account(
"eip155:2:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(is_eip155_account(
"eip155:12:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(!is_eip155_account(
"eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d"
));
assert!(!is_eip155_account(
"eip156:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(!is_eip155_account(
"eip15:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(!is_eip155_account(
"0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(!is_eip155_account(
"eip155:12:62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(!is_eip155_account(
"62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
assert!(!is_eip155_account(
"eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d00"
));
assert!(!is_eip155_account(
"eeip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"
));
fn test_is_valid_eip155_account() {
assert!(
is_valid_eip155_account("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_ok()
);
assert!(
is_valid_eip155_account("eip155:2:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_ok()
);
assert!(
is_valid_eip155_account("eip155:12:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_ok()
);
assert!(
is_valid_eip155_account("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d").is_err()
);
assert!(
is_valid_eip155_account("eip156:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_err()
);
assert!(
is_valid_eip155_account("eip15:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_err()
);
assert!(is_valid_eip155_account("0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_err());
assert!(
is_valid_eip155_account("eip155:12:62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_err()
);
assert!(is_valid_eip155_account("62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_err());
assert!(
is_valid_eip155_account("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d00")
.is_err()
);
assert!(
is_valid_eip155_account("eeip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0")
.is_err()
);
}

#[test]
fn account_id_valid_namespaces() {
assert!(AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_ok());
assert!(AccountId::try_from("solana:xxx").is_err());
}

#[test]
fn account_id_eip155_regex_fail() {
assert_eq!(
AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d"),
Err(AccountIdError::Eip155(Eip155Error::Regex))
);
assert_eq!(
AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d00"),
Err(AccountIdError::Eip155(Eip155Error::Regex))
);
assert_eq!(
AccountId::try_from("eip155:1:"),
Err(AccountIdError::Eip155(Eip155Error::Regex))
);
assert_eq!(
AccountId::try_from("eip155:f:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0"),
Err(AccountIdError::Eip155(Eip155Error::Regex))
);
}

#[test]
fn account_id_eip155_requires_erc_55() {
assert!(AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").is_ok());
assert_eq!(
AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2D0"),
Err(AccountIdError::Eip155(Eip155Error::Erc55))
);
}
}
81 changes: 0 additions & 81 deletions tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,58 +717,6 @@ async fn test_one_subscriber_two_projects() {
}
}

#[tokio::test]
async fn test_account_case_insensitive() {
let (postgres, _) = get_postgres().await;

let topic = Topic::generate();
let project_id = ProjectId::generate();
let subscribe_key = generate_subscribe_key();
let authentication_key = generate_authentication_key();
let app_domain = &generate_app_domain();
upsert_project(
project_id.clone(),
app_domain,
topic,
&authentication_key,
&subscribe_key,
&postgres,
None,
)
.await
.unwrap();
let project = get_project_by_project_id(project_id.clone(), &postgres, None)
.await
.unwrap();

let (_, address) = generate_eoa();
let account = format_eip155_account(1, &address.to_lowercase());
let scope = HashSet::from([Uuid::new_v4(), Uuid::new_v4()]);
let notify_key = rand::Rng::gen::<[u8; 32]>(&mut rand::thread_rng());
let notify_topic = topic_from_key(&notify_key);
upsert_subscriber(
project.id,
account.clone(),
scope,
&notify_key,
notify_topic,
&postgres,
None,
)
.await
.unwrap();

let subscribers = get_subscriptions_by_account_and_maybe_app(
format_eip155_account(1, &address.to_uppercase().replace('X', "x")),
None,
&postgres,
None,
)
.await
.unwrap();
assert_eq!(subscribers.len(), 1);
}

#[tokio::test]
async fn test_get_subscriber_accounts_by_project_id() {
let (postgres, _) = get_postgres().await;
Expand Down Expand Up @@ -6516,35 +6464,6 @@ pub async fn test_same_account() {
&postgres,
)
.await;
test(
true,
&AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").unwrap(),
&AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2D0").unwrap(),
&postgres,
)
.await;
test(
true,
&AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2d0").unwrap(),
&AccountId::try_from("eip155:2:0x62639418051006514eD5Bb5B20aa7aAD642cC2D0").unwrap(),
&postgres,
)
.await;

test(
false,
&AccountId::try_from("eip155:1:0x62639418051006514eD5Bb5B20aa7aAD642cC2e0").unwrap(),
&AccountId::try_from("eip155:2:0x62639418051006514eD5Bb5B20aa7aAD642cC2D0").unwrap(),
&postgres,
)
.await;
test(
false,
&AccountId::try_from("eip155:1:0x52639418051006514eD5Bb5B20aa7aAD642cC2D0").unwrap(),
&AccountId::try_from("eip155:2:0x62639418051006514eD5Bb5B20aa7aAD642cC2D0").unwrap(),
&postgres,
)
.await;
}

#[test_context(NotifyServerContext)]
Expand Down
7 changes: 5 additions & 2 deletions tests/utils/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use {
notify_server::{
auth::{AuthError, DidWeb, GetSharedClaims, SharedClaims},
error::NotifyServerError,
model::types::AccountId,
model::types::{erc_55_checksum_encode, AccountId},
notify_message::NotifyMessage,
relay_client_helpers::create_http_client,
},
Expand Down Expand Up @@ -237,7 +237,10 @@ pub fn generate_eoa() -> (SigningKey, String) {
.as_bytes()[1..],
)
.finalize()[12..];
let address = format!("0x{}", hex::encode(address));
let address = format!(
"0x{}",
erc_55_checksum_encode(&hex::encode(address)).collect::<String>()
);
(account_signing_key, address)
}

Expand Down
Loading