From 0e919f9e255fe1ac4720811a5826ecaaea5da6d5 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Mon, 19 Apr 2021 16:18:29 -0700 Subject: [PATCH 1/3] Fix panic on bad length for SVCB record --- CHANGELOG.md | 6 ++++++ crates/proto/src/rr/rdata/svcb.rs | 13 +++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2582aec16e..772ec5271f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). All notes should be prepended with the location of the change, e.g. `(proto)` or `(resolver)`. +## 0.20.2 + +### Fixed + +- (proto) Panic on bad length in SVCB for record length + ## 0.20.1 ### Added diff --git a/crates/proto/src/rr/rdata/svcb.rs b/crates/proto/src/rr/rdata/svcb.rs index 4e0fed69fc..f97fd110dc 100644 --- a/crates/proto/src/rr/rdata/svcb.rs +++ b/crates/proto/src/rr/rdata/svcb.rs @@ -1032,7 +1032,7 @@ pub fn read(decoder: &mut BinDecoder<'_>, rdata_length: Restrict) -> ProtoR let svc_priority = decoder.read_u16()?.unverified(/*any u16 is valid*/); let target_name = Name::read(decoder)?; - let mut remainder_len = rdata_length.map(|len| len as usize - (decoder.index() - start_index)).unverified(/*valid len*/); + let mut remainder_len = rdata_length.map(|len| len as usize).checked_sub(decoder.index() - start_index).map_err(|len| format!("Bad length for RDATA of SVCB: {}", len))?.unverified(/*valid len*/); let mut svc_params: Vec<(SvcParamKey, SvcParamValue)> = Vec::new(); // must have at least 4 bytes left for the key and the length @@ -1053,7 +1053,7 @@ pub fn read(decoder: &mut BinDecoder<'_>, rdata_length: Restrict) -> ProtoR } svc_params.push((key, value)); - remainder_len = rdata_length.map(|len| len as usize - (decoder.index() - start_index)).unverified(/*valid len*/); + remainder_len = rdata_length.map(|len| len as usize).checked_sub(decoder.index() - start_index).map_err(|len| format!("Bad length for RDATA of SVCB: {}", len))?.unverified(/*valid len*/); } Ok(SVCB { @@ -1211,4 +1211,13 @@ mod tests { ], )); } + + #[test] + fn test_no_panic() { + const BUF: &[u8] = &[ + 255, 121, 0, 0, 0, 0, 40, 255, 255, 160, 160, 0, 0, 0, 64, 0, 1, 255, 158, 0, 0, 0, 8, + 0, 0, 7, 7, 0, 0, 0, 0, 0, 0, 0, + ]; + assert!(crate::op::Message::from_vec(&BUF).is_err()); + } } From e4702028e78604a15acd4e5e9bebaea2826bd198 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Mon, 19 Apr 2021 20:06:38 -0700 Subject: [PATCH 2/3] fix clippy warnings --- Makefile.toml | 2 +- bin/src/named.rs | 2 +- crates/client/src/lib.rs | 2 +- crates/proto/src/lib.rs | 2 +- crates/proto/src/rr/dnssec/rdata/mod.rs | 2 +- crates/proto/src/rr/rdata/opt.rs | 3 ++- crates/proto/src/rr/record_data.rs | 2 +- crates/proto/src/xfer/dns_request.rs | 1 + crates/resolver/src/lib.rs | 1 - crates/resolver/src/system_conf/unix.rs | 2 +- crates/server/src/lib.rs | 2 +- crates/server/src/store/in_memory/authority.rs | 2 +- 12 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 974cc7d861..f4975d6a2c 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -37,7 +37,7 @@ CARGO_MAKE_CRATES_IO_TOKEN = { value = "--token=${CRATES_IO_TOKEN}", condition = [tasks.install-openssl] description = "Installs OpenSSL on Windows" workspace = false -env = { OPENSSL_VERSION = "1_1_1j", OPENSSL_DIR = "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\\target\\OpenSSL" } +env = { OPENSSL_VERSION = "1_1_1k", OPENSSL_DIR = "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\\target\\OpenSSL" } condition = { platforms = ["windows"], files_not_exist = ["${OPENSSL_DIR}"] } script_runner = "powershell" script_extension = "ps1" diff --git a/bin/src/named.rs b/bin/src/named.rs index ffd7242507..a2c2878eac 100644 --- a/bin/src/named.rs +++ b/bin/src/named.rs @@ -488,7 +488,7 @@ fn main() { ); error!("{}", error_msg); - panic!(error_msg); + panic!("{}", error_msg); } }; } diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index cbf7a5d632..ff088126f4 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -28,7 +28,7 @@ )] #![allow( clippy::needless_doctest_main, - clippy::unknown_clippy_lints, + clippy::upper_case_acronyms, clippy::single_component_path_imports )] #![recursion_limit = "1024"] diff --git a/crates/proto/src/lib.rs b/crates/proto/src/lib.rs index 0d488cf74c..6d13e7f03d 100644 --- a/crates/proto/src/lib.rs +++ b/crates/proto/src/lib.rs @@ -18,7 +18,7 @@ rust_2018_idioms, unreachable_pub )] -#![allow(clippy::single_component_path_imports)] +#![allow(clippy::single_component_path_imports, clippy::upper_case_acronyms)] #![recursion_limit = "2048"] //! Trust-DNS Protocol library diff --git a/crates/proto/src/rr/dnssec/rdata/mod.rs b/crates/proto/src/rr/dnssec/rdata/mod.rs index 7ed3f6affb..cb666e3bf5 100644 --- a/crates/proto/src/rr/dnssec/rdata/mod.rs +++ b/crates/proto/src/rr/dnssec/rdata/mod.rs @@ -576,7 +576,7 @@ impl fmt::Display for DNSSECRData { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { fn w(f: &mut fmt::Formatter<'_>, d: D) -> Result<(), fmt::Error> { write!(f, "{rdata}", rdata = d) - }; + } match self { DNSSECRData::DS(ds) => w(f, ds), diff --git a/crates/proto/src/rr/rdata/opt.rs b/crates/proto/src/rr/rdata/opt.rs index cb10c9cb18..fa08ecfeac 100644 --- a/crates/proto/src/rr/rdata/opt.rs +++ b/crates/proto/src/rr/rdata/opt.rs @@ -541,7 +541,8 @@ pub fn test_read_empty_option_at_end_of_opt() { let read_rdata = read(&mut decoder, Restrict::new(bytes.len() as u16)); assert!( read_rdata.is_ok(), - format!("error decoding: {:?}", read_rdata.unwrap_err()) + "error decoding: {:?}", + read_rdata.unwrap_err() ); let opt = read_rdata.unwrap(); diff --git a/crates/proto/src/rr/record_data.rs b/crates/proto/src/rr/record_data.rs index 21196bde44..4a4a069ac3 100644 --- a/crates/proto/src/rr/record_data.rs +++ b/crates/proto/src/rr/record_data.rs @@ -956,7 +956,7 @@ impl fmt::Display for RData { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { fn w(f: &mut fmt::Formatter<'_>, d: D) -> Result<(), fmt::Error> { write!(f, "{rdata}", rdata = d) - }; + } match *self { RData::A(address) => w(f, address), diff --git a/crates/proto/src/xfer/dns_request.rs b/crates/proto/src/xfer/dns_request.rs index 29ed6b045d..427071a881 100644 --- a/crates/proto/src/xfer/dns_request.rs +++ b/crates/proto/src/xfer/dns_request.rs @@ -64,6 +64,7 @@ impl DerefMut for DnsRequest { } } +#[allow(clippy::from_over_into)] impl Into for Message { fn into(self) -> DnsRequest { DnsRequest::new(self, DnsRequestOptions::default()) diff --git a/crates/resolver/src/lib.rs b/crates/resolver/src/lib.rs index 0b050a888e..c9fd77636f 100644 --- a/crates/resolver/src/lib.rs +++ b/crates/resolver/src/lib.rs @@ -182,7 +182,6 @@ #![recursion_limit = "128"] #![allow( clippy::needless_doctest_main, - clippy::unknown_clippy_lints, clippy::single_component_path_imports )] diff --git a/crates/resolver/src/system_conf/unix.rs b/crates/resolver/src/system_conf/unix.rs index 7332674059..d08cf1841f 100644 --- a/crates/resolver/src/system_conf/unix.rs +++ b/crates/resolver/src/system_conf/unix.rs @@ -27,7 +27,7 @@ use crate::proto::rr::Name; const DEFAULT_PORT: u16 = 53; pub fn read_system_conf() -> io::Result<(ResolverConfig, ResolverOpts)> { - Ok(read_resolv_conf("/etc/resolv.conf")?) + read_resolv_conf("/etc/resolv.conf") } fn read_resolv_conf>(path: P) -> io::Result<(ResolverConfig, ResolverOpts)> { diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index 37fbe0e991..cf3d0f9216 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -26,7 +26,7 @@ rust_2018_idioms, unreachable_pub )] -#![allow(clippy::single_component_path_imports)] +#![allow(clippy::upper_case_acronyms, clippy::single_component_path_imports)] #![recursion_limit = "2048"] //! Trust-DNS is intended to be a fully compliant domain name server and client library. diff --git a/crates/server/src/store/in_memory/authority.rs b/crates/server/src/store/in_memory/authority.rs index 5b1536ea82..9782537d8d 100644 --- a/crates/server/src/store/in_memory/authority.rs +++ b/crates/server/src/store/in_memory/authority.rs @@ -382,7 +382,7 @@ impl InMemoryAuthority { (upsert_type == check_type && occupied_type != check_type) || // it's a different record, but there is already a CNAME/ANAME here (upsert_type != check_type && occupied_type == check_type) - }; + } // check that CNAME and ANAME is either not already present, or no other records are if it's a CNAME let start_range_key = From 43167af48dd805d7a9cb0a8e644b5ccc488d1325 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Mon, 19 Apr 2021 20:25:08 -0700 Subject: [PATCH 3/3] fix formatting --- crates/resolver/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/resolver/src/lib.rs b/crates/resolver/src/lib.rs index c9fd77636f..e41ba3ac17 100644 --- a/crates/resolver/src/lib.rs +++ b/crates/resolver/src/lib.rs @@ -180,10 +180,7 @@ unreachable_pub )] #![recursion_limit = "128"] -#![allow( - clippy::needless_doctest_main, - clippy::single_component_path_imports -)] +#![allow(clippy::needless_doctest_main, clippy::single_component_path_imports)] #[cfg(feature = "dns-over-tls")] #[macro_use]