Malformed Ether address could be added
Description
The function verify is used to verify the signature of the transaction. However, the code decodes the hex string from index 2 of the address (skipping the "0x" prefix), effectively ignoring the first two characters. The two bytes of address in Authenticator::EthWallet are not checked for the correct format of the Ethereum address. The address is expected to be a valid Ethereum address, and the address should be 42 characters long and start with 0x.
In this case, the first two bytes could be any value if last 40 bytes are a valid Ethereum address. This could lead to a malformed Ethereum address being added as an authenticator.
pub fn add_auth_method(
deps: DepsMut,
env: &Env,
add_authenticator: &mut AddAuthenticator,
) -> ContractResult<Response> {
// ...
AddAuthenticator::EthWallet {
id,
address,
signature,
} => {
let auth = Authenticator::EthWallet {
address: (*address).clone(),
};
if !auth.verify(
deps.as_ref(),
env,
&Binary::from(env.contract.address.as_bytes()),
signature,
)? {
Err(ContractError::InvalidSignature)
} else {
save_authenticator(deps, *id, &auth)?;
Ok(())
}
}
// contracts/account/src/auth.rs
impl Authenticator {
pub fn verify(
&self,
deps: Deps,
env: &Env,
tx_bytes: &Binary,
sig_bytes: &Binary,
) -> Result<bool, ContractError> {
// ...
Authenticator::EthWallet { address } => {
let addr_bytes: Vec<u8> = hex::decode(&address[2..])?;
match eth_crypto::verify(deps.api, tx_bytes, sig_bytes, &addr_bytes) {
Ok(_) => Ok(true),
Err(error) => Err(error),
}
}Impact
A malformed Ethereum address could be added as an authenticator, such as AAc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2 or ??c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2.
This could confuse the tracker who is tracking the authenticator's address.
Recommendations
Check the address for the correct format of the Ethereum address before adding it as an authenticator.
Remediation
This issue has been acknowledged by Burnt Labs, and a fix was implemented in commit 0e1583eb↗.