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: off by one error during log topic decoding #296
Conversation
@@ -432,7 +432,7 @@ fn derive_decode_from_log_impl( | |||
let flat_topics = topics.iter().skip(1).flat_map(|t| t.as_ref().to_vec()).collect::<Vec<u8>>(); | |||
}, | |||
quote! { | |||
if topic_tokens.is_empty() || topic_tokens.len() != topics.len() - 1 { | |||
if topic_tokens.len() != topics.len() - 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't this subtraction underflow if the LOG0
opcode is used? I'm not super familiar with these codepaths 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro generates different code for anonymous events:
ethers-rs/ethers-contract/ethers-contract-derive/src/lib.rs
Lines 411 to 440 in 94797f5
let (signature_check, flat_topics_init, topic_tokens_len_check) = if event.anonymous { | |
( | |
quote! {}, | |
quote! { | |
let flat_topics = topics.iter().flat_map(|t| t.as_ref().to_vec()).collect::<Vec<u8>>(); | |
}, | |
quote! { | |
if topic_tokens.len() != topics.len() { | |
return Err(ethers_core::abi::Error::InvalidData); | |
} | |
}, | |
) | |
} else { | |
( | |
quote! { | |
let event_signature = topics.get(0).ok_or(ethers_core::abi::Error::InvalidData)?; | |
if event_signature != &Self::signature() { | |
return Err(ethers_core::abi::Error::InvalidData); | |
} | |
}, | |
quote! { | |
let flat_topics = topics.iter().skip(1).flat_map(|t| t.as_ref().to_vec()).collect::<Vec<u8>>(); | |
}, | |
quote! { | |
if topic_tokens.len() != topics.len() - 1 { | |
return Err(ethers_core::abi::Error::InvalidData); | |
} | |
}, | |
) | |
}; |
only for non anonymous it makes sure that the signature is the first entry in topics
let event_signature = topics.get(0).ok_or(ethers_core::abi::Error::InvalidData)?; |
so
topics.len() - 1
can't underflow.
Most of this is adapted from:
https://github.com/rust-ethereum/ethabi/blob/970d47018e0f457440851752a7b46a4dbf793a0b/ethabi/src/event.rs#L122-L173
Motivation
Fixes a bug, that is essentially an off by one error, that resulted in errors when decoding events with no indexed entries.
Solution
Remove the bad
is_empty
requirement and test against real log.