From afb989ced957266c44fd5ee2260c3678a48e9970 Mon Sep 17 00:00:00 2001 From: Gabi Date: Fri, 22 Dec 2023 14:42:32 -0300 Subject: [PATCH] security(connlib): Dont allow acces to non-subdomains for a given resource (#2996) Previously, we just assumed that the domain in the query is a subdomain of the resource but a malicious actor can hijack that field to access domains that doesn't correspond to that resource. With this patch we don't even resolve the address for unrelated domains. --- .../tunnel/src/control_protocol/gateway.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/rust/connlib/tunnel/src/control_protocol/gateway.rs b/rust/connlib/tunnel/src/control_protocol/gateway.rs index 39c4e0593..225fbd49c 100644 --- a/rust/connlib/tunnel/src/control_protocol/gateway.rs +++ b/rust/connlib/tunnel/src/control_protocol/gateway.rs @@ -90,10 +90,16 @@ where } let resource_addresses = match &resource { - ResourceDescription::Dns(_) => { + ResourceDescription::Dns(r) => { let Some(domain) = client_payload.domain.clone() else { return Err(Error::ControlProtocolError); }; + + if !domain.iter_suffixes().any(|d| d.to_string() == r.address) { + let _ = ice.stop().await; + return Err(Error::InvalidResource); + } + (domain.to_string(), 0) .to_socket_addrs()? .map(|addrs| addrs.ip().into()) @@ -170,11 +176,15 @@ where .find(|(_, p)| p.inner.conn_id == client_id) { let addresses = match &resource { - ResourceDescription::Dns(_) => { - let Some(ref domain) = domain else { + ResourceDescription::Dns(r) => { + let Some(domain) = domain.as_ref() else { return None; }; + if !domain.iter_suffixes().any(|d| d.to_string() == r.address) { + return None; + } + (domain.to_string(), 0) .to_socket_addrs() .ok()?