Просмотр исходного кода

net: match on host() instead of host_str() in `block_all_ports`

This fixes a bug where IPv6 addresses could be truncated in different
ways during the comparison due to the `host_str()` conversion and potentially be treated as different addresses.
draoi 2 лет назад
Родитель
Сommit
d63f675fba
3 измененных файлов с 11 добавлено и 8 удалено
  1. 1 1
      src/net/acceptor.rs
  2. 4 2
      src/net/channel.rs
  3. 6 5
      src/net/hosts.rs

+ 1 - 1
src/net/acceptor.rs

@@ -145,7 +145,7 @@ impl Acceptor {
                 Ok((stream, url)) => {
                     // Check if we reject this peer
                     if hosts.container.contains(HostColor::Black as usize, &url) ||
-                        hosts.block_all_ports(url.host_str().unwrap().to_string())
+                        hosts.block_all_ports(url.clone())
                     {
                         warn!(target: "net::acceptor::run_accept_loop()", "Peer {} is blacklisted", url);
                         continue

+ 4 - 2
src/net/channel.rs

@@ -89,7 +89,7 @@ pub struct Channel {
     /// A boolean marking if this channel is stopped
     stopped: AtomicBool,
     /// Weak pointer to respective session
-    session: SessionWeakPtr,
+    pub(in crate::net) session: SessionWeakPtr,
     /// The version message of the node we are connected to.
     /// Some if the version exchange has already occurred, None
     /// otherwise.
@@ -281,7 +281,9 @@ impl Channel {
             return Err(Error::MalformedPacket)
         }
 
+        // First extract the length from the stream
         let cmd_len = VarInt::decode_async(stream).await?.0;
+        // Then extract precisely `cmd_len` items from the stream.
         let mut take = stream.take(cmd_len);
 
         let mut bytes = Vec::new();
@@ -403,7 +405,7 @@ impl Channel {
         // `hosts.block_all_ports()` to true.
         let peer = {
             if self.session_type_id() & SESSION_INBOUND != 0 {
-                if peer.host_str().is_none() {
+                if peer.host().is_none() {
                     error!("[P2P] ban() caught Url without host: {:?}", peer);
                     return
                 }

+ 6 - 5
src/net/hosts.rs

@@ -1136,12 +1136,13 @@ impl Hosts {
     /// hostname in the blacklist. This method will check if a host is
     /// stored in the blacklist without a port, and if so, it will return
     /// true.
-    pub(in crate::net) fn block_all_ports(&self, addr: String) -> bool {
+    pub(in crate::net) fn block_all_ports(&self, url: Url) -> bool {
+        let host = url.host().unwrap();
         self.container.hostlists[HostColor::Black as usize]
             .read()
             .unwrap()
             .iter()
-            .any(|(u, _t)| u.host_str().unwrap() == addr && u.port().is_none())
+            .any(|(u, _t)| u.host().unwrap() == host && u.port().is_none())
     }
 
     /// Filter given addresses based on certain rulesets and validity. Strictly called only on
@@ -1183,7 +1184,7 @@ impl Hosts {
 
             // Blacklist peers should never enter the hostlist.
             if self.container.contains(HostColor::Black as usize, addr_) ||
-                self.block_all_ports(addr_.host_str().unwrap().to_string())
+                self.block_all_ports(addr_.clone())
             {
                 warn!(
                     target: "net::hosts::filter_addresses",
@@ -1481,8 +1482,8 @@ mod tests {
         hosts.container.store(HostColor::Black as usize, blacklist1.clone(), 0);
         hosts.container.store(HostColor::Black as usize, blacklist2.clone(), 0);
 
-        assert!(hosts.block_all_ports(blacklist2.host_str().unwrap().to_string()));
-        assert!(!hosts.block_all_ports(blacklist1.host_str().unwrap().to_string()));
+        assert!(hosts.block_all_ports(blacklist2));
+        assert!(!hosts.block_all_ports(blacklist1));
     }
 
     #[test]