rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C.

Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied
in Rust to the maximum number of version (total, for all subprotocols).
Whereas in C, it was being applied to the number of subprotocols that were
allowed.  This changes the Rust to match C's behaviour.

diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs
index f94e6299c975846906145f134e66530dd35f98f1..4afc50edf8745a4954cfb70c6d3271cc776f694a 100644 (file)
--- a/src/rust/protover/protoset.rs
+++ b/src/rust/protover/protoset.rs
@@ -8,7 +8,6 @@ use std::slice;
 use std::str::FromStr;
 use std::u32;
 
-use protover::MAX_PROTOCOLS_TO_EXPAND;
 use errors::ProtoverError;
 
 /// A single version number.
@@ -183,10 +182,6 @@ impl ProtoSet {
             last_high = high;
         }
 
-        if self.len() > MAX_PROTOCOLS_TO_EXPAND {
-            return Err(ProtoverError::ExceedsMax);
-        }
-
         Ok(self)
     }
 
@@ -317,9 +312,15 @@ impl FromStr for ProtoSet {
     /// assert!(protoset.contains(&5));
     /// assert!(!protoset.contains(&10));
     ///
-    /// // We can also equivalently call `ProtoSet::from_str` by doing:
+    /// // We can also equivalently call `ProtoSet::from_str` by doing (all
+    /// // implementations of `FromStr` can be called this way, this one isn't
+    /// // special):
     /// let protoset: ProtoSet = "4-6,12".parse()?;
     ///
+    /// // Calling it (either way) can take really large ranges (up to `u32::MAX`):
+    /// let protoset: ProtoSet = "1-70000".parse()?;
+    /// let protoset: ProtoSet = "1-4294967296".parse()?;
+    ///
     /// // There are lots of ways to get an `Err` from this function.  Here are
     /// // a few:
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("="));
@@ -327,7 +328,6 @@ impl FromStr for ProtoSet {
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int"));
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-"));
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4"));
-    /// assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("1-70000"));
     ///
     /// // Things which would get parsed into an _empty_ `ProtoSet` are,
     /// // however, legal, and result in an empty `ProtoSet`:

diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index fc89f70d4ce763b5a2945492d622348da8ae19db..5e5a31cd333aa8209c152466427e63077c36c41b 100644 (file)
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -26,7 +26,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha";
 /// before concluding that someone is trying to DoS us
 ///
 /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND`
-pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16);
+const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16);
 
 /// Currently supported protocols and their versions, as a byte-slice.
 ///
@@ -166,6 +166,10 @@ impl ProtoEntry {
         supported.parse()
     }
 
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
     pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> {
         self.0.get(protocol)
     }
@@ -220,8 +224,11 @@ impl FromStr for ProtoEntry {
             let proto_name: Protocol = proto.parse()?;
 
             proto_entry.insert(proto_name, versions);
-        }
 
+            if proto_entry.len() > MAX_PROTOCOLS_TO_EXPAND {
+                return Err(ProtoverError::ExceedsMax);
+            }
+        }
         Ok(proto_entry)
     }
 }
@@ -737,9 +744,14 @@ mod test {
         assert_protoentry_is_unparseable!("Ducks=5-7,8");
     }
 
+    #[test]
+    fn test_protoentry_from_str_allowed_number_of_versions() {
+        assert_protoentry_is_parseable!("Desc=1-4294967294");
+    }
+
     #[test]
     fn test_protoentry_from_str_too_many_versions() {
-        assert_protoentry_is_unparseable!("Desc=1-65537");
+        assert_protoentry_is_unparseable!("Desc=1-4294967295");
     }
 
     #[test]

diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs
index 11015f35b4ed7c316a3d2a91df80607150e40f3a..2db01a1634416f3c2496dee687bad80e4a96f5ba 100644 (file)
--- a/src/rust/protover/tests/protover.rs
+++ b/src/rust/protover/tests/protover.rs
@@ -353,13 +353,6 @@ fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() {
 }
 
 #[test]
-#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")]
-// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an
-// UnvalidatedProtoEntry is defined as a Hashmap<UnknownProtocol, ProtoSet>.
-// Because it contains the ProtoSet part, there's still *some* validation
-// happening, so in this case the DoS protections in the Rust code are kicking
-// in because the range here is exceeds the maximum, so it returns an
-// Err(ProtoverError::ExceedsMax).  The C, however seems to parse it anyway.
 fn protover_all_supported_should_not_dos_anyones_computer() {
     let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();
@@ -368,13 +361,6 @@ fn protover_all_supported_should_not_dos_anyones_computer() {
 }
 
 #[test]
-#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")]
-// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an
-// UnvalidatedProtoEntry is defined as a Hashmap<UnknownProtocol, ProtoSet>.
-// Because it contains the ProtoSet part, there's still *some* validation
-// happening, so in this case the DoS protections in the Rust code are kicking
-// in because the range here is exceeds the maximum, so it returns an
-// Err(ProtoverError::ExceedsMax).  The C, however seems to parse it anyway.
 fn protover_all_supported_should_not_dos_anyones_computer_max_versions() {
     let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();