From d46a19098ae0450635beb5150538bdb2c87b8624 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2020 19:26:26 +0000 Subject: [PATCH] Bound ip-echo-server reply read (bp #13543) (#13546) * ip-echo-server: Name the header length magic number (cherry picked from commit aab5f24518a1cd3b619b06db32e65c5910384787) * ip-echo-server: Add helper to compute reply length (cherry picked from commit 7481ba561822ae1a75029bc53f79cb7f4a892f93) * ip-echo-server: Limit socket read to expected reply length (cherry picked from commit d2cfeb31b935307d82940ceb938bcd4cc179dacf) Co-authored-by: Trent Nelson --- net-utils/src/ip_echo_server.rs | 19 +++++++++---------- net-utils/src/lib.rs | 20 ++++++++++++++------ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/net-utils/src/ip_echo_server.rs b/net-utils/src/ip_echo_server.rs index 7a3d17967d..4ba62a6ea1 100644 --- a/net-utils/src/ip_echo_server.rs +++ b/net-utils/src/ip_echo_server.rs @@ -1,3 +1,4 @@ +use crate::{ip_echo_server_reply_length, HEADER_LENGTH}; use bytes::Bytes; use log::*; use serde_derive::{Deserialize, Serialize}; @@ -52,13 +53,14 @@ pub fn ip_echo_server(tcp: std::net::TcpListener) -> IpEchoServer { let processor = reader .and_then(move |data| { - if data.len() < 4 { + if data.len() < HEADER_LENGTH { return Err(io::Error::new( io::ErrorKind::Other, format!("Request too short, received {} bytes", data.len()), )); } - let request_header: String = data[0..4].iter().map(|b| *b as char).collect(); + let request_header: String = + data[0..HEADER_LENGTH].iter().map(|b| *b as char).collect(); if request_header != "\0\0\0\0" { // Explicitly check for HTTP GET/POST requests to more gracefully handle // the case where a user accidentally tried to use a gossip entrypoint in @@ -74,7 +76,7 @@ pub fn ip_echo_server(tcp: std::net::TcpListener) -> IpEchoServer { let expected_len = bincode::serialized_size(&IpEchoServerMessage::default()).unwrap() as usize; - let actual_len = data[4..].len(); + let actual_len = data[HEADER_LENGTH..].len(); if actual_len < expected_len { return Err(io::Error::new( io::ErrorKind::Other, @@ -85,7 +87,7 @@ pub fn ip_echo_server(tcp: std::net::TcpListener) -> IpEchoServer { )); } - bincode::deserialize::(&data[4..]) + bincode::deserialize::(&data[HEADER_LENGTH..]) .map(Some) .map_err(|err| { io::Error::new( @@ -175,12 +177,9 @@ pub fn ip_echo_server(tcp: std::net::TcpListener) -> IpEchoServer { } else { // "\0\0\0\0" header is added to ensure a valid response will never // conflict with the first four bytes of a valid HTTP response. - let mut bytes = vec![ - 0; - 4 + bincode::serialized_size(&peer_addr.ip()).unwrap() - as usize - ]; - bincode::serialize_into(&mut bytes[4..], &peer_addr.ip()).unwrap(); + let mut bytes = vec![0u8; ip_echo_server_reply_length()]; + bincode::serialize_into(&mut bytes[HEADER_LENGTH..], &peer_addr.ip()) + .unwrap(); Ok(Bytes::from(bytes)) } }); diff --git a/net-utils/src/lib.rs b/net-utils/src/lib.rs index 71ecffe418..75dd997221 100644 --- a/net-utils/src/lib.rs +++ b/net-utils/src/lib.rs @@ -22,16 +22,23 @@ pub struct UdpSocketPair { pub type PortRange = (u16, u16); +pub(crate) const HEADER_LENGTH: usize = 4; +pub(crate) fn ip_echo_server_reply_length() -> usize { + let largest_ip_addr = IpAddr::from([0u16; 8]); // IPv6 variant + HEADER_LENGTH + bincode::serialized_size(&largest_ip_addr).unwrap() as usize +} + fn ip_echo_server_request( ip_echo_server_addr: &SocketAddr, msg: IpEchoServerMessage, ) -> Result { - let mut data = Vec::new(); + let mut data = vec![0u8; ip_echo_server_reply_length()]; let timeout = Duration::new(5, 0); TcpStream::connect_timeout(ip_echo_server_addr, timeout) .and_then(|mut stream| { - let mut bytes = vec![0; 4]; // Start with 4 null bytes to avoid looking like an HTTP GET/POST request + // Start with HEADER_LENGTH null bytes to avoid looking like an HTTP GET/POST request + let mut bytes = vec![0; HEADER_LENGTH]; bytes.append(&mut bincode::serialize(&msg).expect("serialize IpEchoServerMessage")); @@ -42,20 +49,21 @@ fn ip_echo_server_request( stream.set_read_timeout(Some(Duration::new(10, 0)))?; stream.write_all(&bytes)?; stream.shutdown(std::net::Shutdown::Write)?; - stream.read_to_end(&mut data) + stream.read(data.as_mut_slice()) }) .and_then(|_| { // It's common for users to accidentally confuse the validator's gossip port and JSON // RPC port. Attempt to detect when this occurs by looking for the standard HTTP // response header and provide the user with a helpful error message - if data.len() < 4 { + if data.len() < HEADER_LENGTH { return Err(io::Error::new( io::ErrorKind::Other, format!("Response too short, received {} bytes", data.len()), )); } - let response_header: String = data[0..4].iter().map(|b| *b as char).collect(); + let response_header: String = + data[0..HEADER_LENGTH].iter().map(|b| *b as char).collect(); if response_header != "\0\0\0\0" { if response_header == "HTTP" { let http_response = data.iter().map(|b| *b as char).collect::(); @@ -76,7 +84,7 @@ fn ip_echo_server_request( )); } - bincode::deserialize(&data[4..]).map_err(|err| { + bincode::deserialize(&data[HEADER_LENGTH..]).map_err(|err| { io::Error::new( io::ErrorKind::Other, format!("Failed to deserialize: {:?}", err),