p2p: Implement `encoding` traits for `AddrV2Message`
What the developer wrote
Authored by rustaceanrob
p2p: Implement `encoding` traits for `AddrV2Message`
Member of `AddrV2Payload` which is ultimately apart of
`NetworkMessage`.
Some non-zero number of clients advertise services outside of 32 bits of
precision, which requires decoding arbitrarily large compact sizes. As
these integers do not represent collection lengths and will not be used
for allocation, this is acceptable.
ref: https://github.com/bitcoin/bitcoin/blob/4d7d5f6b79d4c11c47e7a828d81296918fd11d4d/src/protocol.h#L332
ref: https://github.com/bitcoin/bitcoin/issues/34768
What changed, and why it matters
This commit adds encoding and decoding support for a Bitcoin peer-to-peer network address message type (AddrV2Message). It changes how the 'services' field is decoded so it can accept very large numbers sent by some real Bitcoin nodes, instead of being limited to 32-bit values. The commit message says this is safe because the value is not used to allocate memory. There is no direct evidence in the commit that this fixes a security vulnerability, but handling unexpectedly large input values is a common place where bugs can occur.
Review whether ServiceFlags correctly rejects or masks service bits above the protocol-defined range, and verify that downstream code does not later use the raw u64 as a length, bitmask index, or allocation trigger. Consider adding unit tests with malformed compact-size service values at the boundary.
Security signals we found
Change in deserialization boundary: widens accepted integer range for untrusted network input
Commit explicitly discusses decoding 'arbitrarily large compact sizes' from peer messages
References upstream Bitcoin Core issue #34768 about non-standard service flag advertisement
No bounds/validation logic for services value beyond wrapping in ServiceFlags
No explicit security framing or CVE/fix language in commit message
Evidence from the diff
The patch implements Encodable/Decodable traits for AddrV2Message in rust-bitcoin’s p2p module. The key change is switching the services field decoder from a 32-bit compact size to CompactSizeU64Decoder, allowing arbitrarily large compact-size integers. The commit references Bitcoin Core protocol.h and issue bitcoin/bitcoin#34768, noting some clients advertise service bits beyond 32 bits. The value is wrapped in ServiceFlags and not used as a collection length, so the authors argue allocation risks are absent. The change is framed as protocol compatibility rather than a security fix.
Changed components
rust-bitcoin p2p address handling (p2p/src/address.rs)AddrV2Message encoder/decoderAddrV2MessageInnerDecoder / CompactSizeU64DecoderInspect captured patch +80 / −2
diff --git a/p2p/src/address.rs b/p2p/src/address.rs
index 53add048..9c6a4758 100644
--- a/p2p/src/address.rs
+++ b/p2p/src/address.rs
@@ -15,8 +15,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV
use arbitrary::{Arbitrary, Unstructured};
use bitcoin::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt};
use encoding::{
- ArrayDecoder, ArrayEncoder, ByteVecDecoder, BytesEncoder, CompactSizeEncoder, Decoder2,
- Encoder2,
+ ArrayDecoder, ArrayEncoder, ByteVecDecoder, BytesEncoder, CompactSizeEncoder, CompactSizeU64Decoder, Decoder2, Decoder4, Encoder2, Encoder4
};
use internals::array::ArrayExt;
use internals::write_err;
@@ -850,6 +849,85 @@ impl ToSocketAddrs for AddrV2Message {
}
}
+encoding::encoder_newtype! {
+ /// The encoder type for an [`AddrV2Message`].
+ pub struct AddrV2MessageEncoder<'e>(Encoder4<ArrayEncoder<4>, CompactSizeEncoder, AddrV2Encoder<'e>, ArrayEncoder<2>>);
+}
+
+impl encoding::Encodable for AddrV2Message {
+ type Encoder<'e> = AddrV2MessageEncoder<'e>;
+
+ fn encoder(&self) -> Self::Encoder<'_> {
+ AddrV2MessageEncoder::new(
+ Encoder4::new(
+ ArrayEncoder::without_length_prefix(self.time.to_le_bytes()),
+ CompactSizeEncoder::new_u64(self.services.to_u64()),
+ self.addr.encoder(),
+ ArrayEncoder::without_length_prefix(self.port.to_be_bytes())
+ )
+ )
+ }
+}
+
+type AddrV2MessageInnerDecoder = Decoder4<ArrayDecoder<4>, CompactSizeU64Decoder, AddrV2Decoder, ArrayDecoder<2>>;
+
+/// The decoder for an [`AddrV2Message`].
+pub struct AddrV2MessageDecoder(AddrV2MessageInnerDecoder);
+
+impl encoding::Decoder for AddrV2MessageDecoder {
+ type Output = AddrV2Message;
+ type Error = AddrV2MessageDecoderError;
+
+ #[inline]
+ fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
+ self.0.push_bytes(bytes).map_err(AddrV2MessageDecoderError)
+ }
+
+ #[inline]
+ fn end(self) -> Result<Self::Output, Self::Error> {
+ let (time, services, addr, port) = self.0.end().map_err(AddrV2MessageDecoderError)?;
+ let services = ServiceFlags(services);
+ let time = u32::from_le_bytes(time);
+ let port = u16::from_be_bytes(port);
+ Ok(AddrV2Message { time, services, addr, port })
+ }
+
+ #[inline]
+ fn read_limit(&self) -> usize { self.0.read_limit() }
+}
+
+impl encoding::Decodable for AddrV2Message {
+ type Decoder = AddrV2MessageDecoder;
+
+ fn decoder() -> Self::Decoder {
+ AddrV2MessageDecoder(AddrV2MessageInnerDecoder::new(
+ ArrayDecoder::new(),
+ CompactSizeU64Decoder::new(),
+ AddrV2::decoder(),
+ ArrayDecoder::new())
+ )
+ }
+}
+
+/// An error occuring when decoding a [`AddrV2Message`].
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct AddrV2MessageDecoderError(<AddrV2MessageInnerDecoder as encoding::Decoder>::Error);
+
+impl From<Infallible> for AddrV2MessageDecoderError {
+ fn from(never: Infallible) -> Self { match never {} }
+}
+
+impl fmt::Display for AddrV2MessageDecoderError {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write_err!(f, "addrv2 message error"; self.0)
+ }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for AddrV2MessageDecoderError {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
/// Error returned when an address cannot be converted to an IP-based address.
///
/// Addresses like Tor, I2P, and CJDNS use different routing mechanisms
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.