fix(permissions): handle ipv6 addresses correctly (#24397)

Also don't panic on invalid domain names and addresses. Extracted with cleanups up from #24080 Co-authored-by: Yazan AbdAl-Rahman <[email protected]>
denoland · Jul 5, 2024 · 74ac29b · 74ac29b
1 parent 80df9ae
commit 74ac29b
Show file tree

Hide file tree

Showing 11 changed files with 415 additions and 138 deletions.
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -7859,7 7859,7 @@ mod tests {
  let r = flags_from_vec(svec![
  "deno",
  "run",
- "--unsafely-ignore-certificate-errors=deno.land,localhost,::,127.0.0.1,[::1],1.2.3.4",
  "--unsafely-ignore-certificate-errors=deno.land,localhost,[::],127.0.0.1,[::1],1.2.3.4",
  "script.ts"
  ]);
  assert_eq!(
@@ -7871,7 7871,7 @@ mod tests {
  unsafely_ignore_certificate_errors: Some(svec![
  "deno.land",
  "localhost",
- "::",
  "[::]",
  "127.0.0.1",
  "[::1]",
  "1.2.3.4"
@@ -7887,7 7887,7 @@ mod tests {
  let r = flags_from_vec(svec![
  "deno",
  "repl",
- "--unsafely-ignore-certificate-errors=deno.land,localhost,::,127.0.0.1,[::1],1.2.3.4"]);
  "--unsafely-ignore-certificate-errors=deno.land,localhost,[::],127.0.0.1,[::1],1.2.3.4"]);
  assert_eq!(
  r.unwrap(),
  Flags {
@@ -7899,7 7899,7 @@ mod tests {
  unsafely_ignore_certificate_errors: Some(svec![
  "deno.land",
  "localhost",
- "::",
  "[::]",
  "127.0.0.1",
  "[::1]",
  "1.2.3.4"
@@ -8091,7 8091,7 @@ mod tests {
  let r = flags_from_vec(svec![
  "deno",
  "run",
- "--allow-net=deno.land,deno.land:80,::,127.0.0.1,[::1],1.2.3.4:5678,:5678,[::1]:8080",
  "--allow-net=deno.land,deno.land:80,[::],127.0.0.1,[::1],1.2.3.4:5678,:5678,[::1]:8080",
  "script.ts"
  ]);
  assert_eq!(
@@ -8104,7 8104,7 @@ mod tests {
  allow_net: Some(svec![
  "deno.land",
  "deno.land:80",
- "::",
  "[::]",
  "127.0.0.1",
  "[::1]",
  "1.2.3.4:5678",
@@ -8126,7 8126,7 @@ mod tests {
  let r = flags_from_vec(svec![
  "deno",
  "run",
- "--deny-net=deno.land,deno.land:80,::,127.0.0.1,[::1],1.2.3.4:5678,:5678,[::1]:8080",
  "--deny-net=deno.land,deno.land:80,[::],127.0.0.1,[::1],1.2.3.4:5678,:5678,[::1]:8080",
  "script.ts"
  ]);
  assert_eq!(
@@ -8139,7 8139,7 @@ mod tests {
  deny_net: Some(svec![
  "deno.land",
  "deno.land:80",
- "::",
  "[::]",
  "127.0.0.1",
  "[::1]",
  "1.2.3.4:5678",

diff --git a/cli/args/flags_net.rs b/cli/args/flags_net.rs
@@ -1,6 1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use deno_core::url::Url;
 use deno_runtime::deno_permissions::NetDescriptor;
 use std::net::IpAddr;
 use std::str::FromStr;
 
@@ -42,21 43,17 @@ pub fn validator(host_and_port: &str) -> Result<String, String> {
 /// `127.0.0.1:port` and `localhost:port`.
 pub fn parse(paths: Vec<String>) -> clap::error::Result<Vec<String>> {
  let mut out: Vec<String> = vec![];
- for host_and_port in paths.iter() {
- if Url::parse(&format!("internal://{host_and_port}")).is_ok()
- || host_and_port.parse::<IpAddr>().is_ok()
- {
- out.push(host_and_port.to_owned())
- } else if let Ok(port) = host_and_port.parse::<BarePort>() {
  for host_and_port in paths.into_iter() {
  if let Ok(port) = host_and_port.parse::<BarePort>() {
  // we got bare port, let's add default hosts
  for host in ["0.0.0.0", "127.0.0.1", "localhost"].iter() {
  out.push(format!("{}:{}", host, port.0));
  }
  } else {
- return Err(clap::Error::raw(
- clap::error::ErrorKind::InvalidValue,
-  format!("Bad host:port pair: {host_and_port}"),
- ));
  host_and_port.parse::<NetDescriptor>().map_err(|e| {
  clap::Error::raw(clap::error::ErrorKind::InvalidValue, format!("{e:?}"))
  })?;
  out.push(host_and_port)
  }
  }
  Ok(out)
@@ -121,8 118,8 @@ mod tests {
  let entries = svec![
  "deno.land",
  "deno.land:80",
- "::",
- "::1",
  "[::]",
  "[::1]",
  "127.0.0.1",
  "[::1]",
  "1.2.3.4:5678",
@@ -142,8 139,8 @@ mod tests {
  let expected = svec![
  "deno.land",
  "deno.land:80",
- "::",
- "::1",
  "[::]",
  "[::1]",
  "127.0.0.1",
  "[::1]",
  "1.2.3.4:5678",
@@ -174,10 171,8 @@ mod tests {
 
  #[test]
  fn parse_net_args_ipv6() {
- let entries =
- svec!["::", "::1", "[::1]", "[::]:5678", "[::1]:5678", "::cafe"];
- let expected =
- svec!["::", "::1", "[::1]", "[::]:5678", "[::1]:5678", "::cafe"];
  let entries = svec!["[::1]", "[::]:5678", "[::1]:5678"];
  let expected = svec!["[::1]", "[::]:5678", "[::1]:5678"];
  let actual = parse(entries).unwrap();
  assert_eq!(actual, expected);
  }
@@ -190,12 185,36 @@ mod tests {
 
  #[test]
  fn parse_net_args_ipv6_error2() {
- let entries = svec!["0123:4567:890a:bcde:fg::"];
  let entries = svec!["::1"];
  assert!(parse(entries).is_err());
  }
 
  #[test]
  fn parse_net_args_ipv6_error3() {
  let entries = svec!["::"];
  assert!(parse(entries).is_err());
  }
 
  #[test]
  fn parse_net_args_ipv6_error4() {
  let entries = svec!["::cafe"];
  assert!(parse(entries).is_err());
  }
 
  #[test]
  fn parse_net_args_ipv6_error5() {
  let entries = svec!["1::1"];
  assert!(parse(entries).is_err());
  }
 
  #[test]
  fn parse_net_args_ipv6_error6() {
  let entries = svec!["0123:4567:890a:bcde:fg::"];
  assert!(parse(entries).is_err());
  }
 
  #[test]
  fn parse_net_args_ipv6_error7() {
  let entries = svec!["[::q]:8080"];
  assert!(parse(entries).is_err());
  }

diff --git a/runtime/ops/permissions.rs b/runtime/ops/permissions.rs
@@ -4,10 4,8 @@ use ::deno_permissions::parse_sys_kind;
 use ::deno_permissions::PermissionState;
 use ::deno_permissions::PermissionsContainer;
 use deno_core::error::custom_error;
-use deno_core::error::uri_error;
 use deno_core::error::AnyError;
 use deno_core::op2;
-use deno_core::url;
 use deno_core::OpState;
 use serde::Deserialize;
 use serde::Serialize;
@@ -65,7 63,7 @@ pub fn op_query_permission(
  "net" => permissions.net.query(
  match args.host.as_deref() {
  None => None,
- Some(h) => Some(parse_host(h)?),
  Some(h) => Some(h.parse()?),
  }
  .as_ref(),
  ),
@@ -100,7 98,7 @@ pub fn op_revoke_permission(
  "net" => permissions.net.revoke(
  match args.host.as_deref() {
  None => None,
- Some(h) => Some(parse_host(h)?),
  Some(h) => Some(h.parse()?),
  }
  .as_ref(),
  ),
@@ -135,7 133,7 @@ pub fn op_request_permission(
  "net" => permissions.net.request(
  match args.host.as_deref() {
  None => None,
- Some(h) => Some(parse_host(h)?),
  Some(h) => Some(h.parse()?),
  }
  .as_ref(),
  ),
@@ -155,13 153,3 @@ pub fn op_request_permission(
  };
  Ok(PermissionStatus::from(perm))
 }
-
-fn parse_host(host_str: &str) -> Result<(String, Option<u16>), AnyError> {
- let url = url::Url::parse(&format!("http://{host_str}/"))
- .map_err(|_| uri_error("Invalid host"))?;
- if url.path() != "/" {
- return Err(uri_error("Invalid host"));
- }
- let hostname = url.host_str().unwrap();
- Ok((hostname.to_string(), url.port()))
-}