Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic on bad length for SVCB record #1465

Merged
merged 3 commits into from Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile.toml
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion bin/src/named.rs
Expand Up @@ -488,7 +488,7 @@ fn main() {
);

error!("{}", error_msg);
panic!(error_msg);
panic!("{}", error_msg);
}
};
}
Expand Down
2 changes: 1 addition & 1 deletion crates/client/src/lib.rs
Expand Up @@ -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"]
Expand Down
2 changes: 1 addition & 1 deletion crates/proto/src/lib.rs
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/proto/src/rr/dnssec/rdata/mod.rs
Expand Up @@ -576,7 +576,7 @@ impl fmt::Display for DNSSECRData {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
fn w<D: fmt::Display>(f: &mut fmt::Formatter<'_>, d: D) -> Result<(), fmt::Error> {
write!(f, "{rdata}", rdata = d)
};
}

match self {
DNSSECRData::DS(ds) => w(f, ds),
Expand Down
3 changes: 2 additions & 1 deletion crates/proto/src/rr/rdata/opt.rs
Expand Up @@ -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();
Expand Down
13 changes: 11 additions & 2 deletions crates/proto/src/rr/rdata/svcb.rs
Expand Up @@ -1032,7 +1032,7 @@ pub fn read(decoder: &mut BinDecoder<'_>, rdata_length: Restrict<u16>) -> 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
Expand All @@ -1053,7 +1053,7 @@ pub fn read(decoder: &mut BinDecoder<'_>, rdata_length: Restrict<u16>) -> 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 {
Expand Down Expand Up @@ -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());
}
}
2 changes: 1 addition & 1 deletion crates/proto/src/rr/record_data.rs
Expand Up @@ -956,7 +956,7 @@ impl fmt::Display for RData {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
fn w<D: fmt::Display>(f: &mut fmt::Formatter<'_>, d: D) -> Result<(), fmt::Error> {
write!(f, "{rdata}", rdata = d)
};
}

match *self {
RData::A(address) => w(f, address),
Expand Down
1 change: 1 addition & 0 deletions crates/proto/src/xfer/dns_request.rs
Expand Up @@ -64,6 +64,7 @@ impl DerefMut for DnsRequest {
}
}

#[allow(clippy::from_over_into)]
impl Into<DnsRequest> for Message {
fn into(self) -> DnsRequest {
DnsRequest::new(self, DnsRequestOptions::default())
Expand Down
6 changes: 1 addition & 5 deletions crates/resolver/src/lib.rs
Expand Up @@ -180,11 +180,7 @@
unreachable_pub
)]
#![recursion_limit = "128"]
#![allow(
clippy::needless_doctest_main,
clippy::unknown_clippy_lints,
clippy::single_component_path_imports
)]
#![allow(clippy::needless_doctest_main, clippy::single_component_path_imports)]

#[cfg(feature = "dns-over-tls")]
#[macro_use]
Expand Down
2 changes: 1 addition & 1 deletion crates/resolver/src/system_conf/unix.rs
Expand Up @@ -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<P: AsRef<Path>>(path: P) -> io::Result<(ResolverConfig, ResolverOpts)> {
Expand Down
2 changes: 1 addition & 1 deletion crates/server/src/lib.rs
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion crates/server/src/store/in_memory/authority.rs
Expand Up @@ -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 =
Expand Down