diff options
| -rw-r--r-- | src/rust/protover/protoset.rs | 14 | ||||
| -rw-r--r-- | src/rust/protover/protover.rs | 18 | ||||
| -rw-r--r-- | src/rust/protover/tests/protover.rs | 14 |
3 files changed, 22 insertions, 24 deletions
diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs index f94e6299c9..4afc50edf8 100644 --- 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 fc89f70d4c..5e5a31cd33 100644 --- 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) } } @@ -738,8 +745,13 @@ mod test { } #[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 11015f35b4..2db01a1634 100644 --- 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(); |