diff --git a/src/hbbs_http.rs b/src/hbbs_http.rs index 9e4538697..e33c811d4 100644 --- a/src/hbbs_http.rs +++ b/src/hbbs_http.rs @@ -9,8 +9,8 @@ mod http_client; pub mod record_upload; pub mod sync; pub use http_client::{ - create_http_client_async, create_http_client_async_with_url, create_http_client_with_url, - get_url_for_tls, + create_http_client_async, create_http_client_async_with_url_strict, + create_http_client_with_url, create_http_client_with_url_strict, get_url_for_tls, }; #[derive(Debug)] diff --git a/src/hbbs_http/downloader.rs b/src/hbbs_http/downloader.rs index 573e7e77c..a4fd1fa32 100644 --- a/src/hbbs_http/downloader.rs +++ b/src/hbbs_http/downloader.rs @@ -1,4 +1,4 @@ -use super::create_http_client_async_with_url; +use super::create_http_client_async_with_url_strict; use hbb_common::{ bail, lazy_static::lazy_static, @@ -167,7 +167,7 @@ async fn do_download( auto_del_dur: Option, mut rx_cancel: UnboundedReceiver<()>, ) -> ResultType { - let client = create_http_client_async_with_url(&url).await; + let client = create_http_client_async_with_url_strict(&url).await?; let mut is_all_downloaded = false; tokio::select! { diff --git a/src/hbbs_http/http_client.rs b/src/hbbs_http/http_client.rs index 432e5fa38..4e3405492 100644 --- a/src/hbbs_http/http_client.rs +++ b/src/hbbs_http/http_client.rs @@ -1,5 +1,6 @@ use hbb_common::{ async_recursion::async_recursion, + bail, config::{Config, Socks5Server}, log::{self, info}, proxy::{Proxy, ProxyScheme}, @@ -7,6 +8,7 @@ use hbb_common::{ get_cached_tls_accept_invalid_cert, get_cached_tls_type, is_plain, upsert_tls_cache, TlsType, }, + ResultType, }; use reqwest::{blocking::Client as SyncClient, Client as AsyncClient}; @@ -137,6 +139,32 @@ pub fn create_http_client_with_url(url: &str) -> SyncClient { ) } +pub fn create_http_client_with_url_strict(url: &str) -> ResultType { + let parsed_url = url::Url::parse(url)?; + if parsed_url.scheme() != "https" { + bail!("Strict HTTP client requires HTTPS: {}", url); + } + let proxy_conf = Config::get_socks(); + let tls_url = get_url_for_tls(url, &proxy_conf); + let cached_tls_type = get_cached_tls_type(tls_url); + let cached_danger_accept_invalid_cert = get_cached_tls_accept_invalid_cert(tls_url); + let can_reuse_cached_probe = + cached_tls_type.is_some() && cached_danger_accept_invalid_cert == Some(false); + let tls_type = if can_reuse_cached_probe { + cached_tls_type.unwrap_or(TlsType::Rustls) + } else { + TlsType::Rustls + }; + Ok(create_http_client_with_url_( + url, + tls_url, + tls_type, + can_reuse_cached_probe, + Some(false), + Some(false), + )) +} + fn create_http_client_with_url_( url: &str, tls_url: &str, @@ -247,6 +275,33 @@ pub async fn create_http_client_async_with_url(url: &str) -> AsyncClient { .await } +pub async fn create_http_client_async_with_url_strict(url: &str) -> ResultType { + let parsed_url = url::Url::parse(url)?; + if parsed_url.scheme() != "https" { + bail!("Strict HTTP client requires HTTPS: {}", url); + } + let proxy_conf = Config::get_socks(); + let tls_url = get_url_for_tls(url, &proxy_conf); + let cached_tls_type = get_cached_tls_type(tls_url); + let cached_danger_accept_invalid_cert = get_cached_tls_accept_invalid_cert(tls_url); + let can_reuse_cached_probe = + cached_tls_type.is_some() && cached_danger_accept_invalid_cert == Some(false); + let tls_type = if can_reuse_cached_probe { + cached_tls_type.unwrap_or(TlsType::Rustls) + } else { + TlsType::Rustls + }; + Ok(create_http_client_async_with_url_( + url, + tls_url, + tls_type, + can_reuse_cached_probe, + Some(false), + Some(false), + ) + .await) +} + #[async_recursion] async fn create_http_client_async_with_url_( url: &str, diff --git a/src/updater.rs b/src/updater.rs index 56fdc1d2a..bf923dd56 100644 --- a/src/updater.rs +++ b/src/updater.rs @@ -1,8 +1,8 @@ -use crate::{common::do_check_software_update, hbbs_http::create_http_client_with_url}; +use crate::{common::do_check_software_update, hbbs_http::create_http_client_with_url_strict}; use hbb_common::{bail, config, log, ResultType}; use std::{ io::Write, - path::PathBuf, + path::{Component, Path, PathBuf}, sync::{ atomic::{AtomicUsize, Ordering}, mpsc::{channel, Receiver, Sender}, @@ -153,7 +153,7 @@ fn check_update(manually: bool) -> ResultType<()> { format!("{}/rustdesk-{}-x86-sciter.exe", download_url, version) }; log::debug!("New version available: {}", &version); - let client = create_http_client_with_url(&download_url); + let client = create_http_client_with_url_strict(&download_url)?; let Some(file_path) = get_download_file_from_url(&download_url) else { bail!("Failed to get the file path from the URL: {}", download_url); }; @@ -291,7 +291,96 @@ fn update_new_version(update_msi: bool, version: &str, file_path: &PathBuf) { } } -pub fn get_download_file_from_url(url: &str) -> Option { - let filename = url.split('/').last()?; +pub fn get_update_download_file_from_url(url: &str) -> Option { + let parsed = url::Url::parse(url).ok()?; + // Check the raw prefix before Url normalizes default ports. + if !url.starts_with("https://github.com/") + || parsed.scheme() != "https" + || parsed.host_str() != Some("github.com") + || !parsed.username().is_empty() + || parsed.password().is_some() + || parsed.port().is_some() + || parsed.query().is_some() + || parsed.fragment().is_some() + { + return None; + } + + let mut segments = parsed.path_segments()?; + let owner = segments.next()?; + let repo = segments.next()?; + let releases = segments.next()?; + let download = segments.next()?; + let tag = segments.next()?; + let filename = segments.next()?; + + if owner != "rustdesk" + || repo != "rustdesk" + || releases != "releases" + || download != "download" + || tag.is_empty() + || segments.next().is_some() + || !is_plain_update_filename(filename) + { + return None; + } + Some(std::env::temp_dir().join(filename)) } + +fn is_plain_update_filename(filename: &str) -> bool { + if filename.is_empty() + || filename.contains('/') + || filename.contains('\\') + || filename.contains(':') + { + return false; + } + + let mut components = Path::new(filename).components(); + matches!( + components.next(), + Some(Component::Normal(name)) if name.to_str() == Some(filename) + ) && components.next().is_none() +} + +pub fn get_download_file_from_url(url: &str) -> Option { + get_update_download_file_from_url(url) +} + +#[cfg(test)] +mod tests { + use super::get_download_file_from_url; + + #[test] + fn update_download_file_accepts_expected_github_asset_urls() { + let file = get_download_file_from_url( + "https://github.com/rustdesk/rustdesk/releases/download/1.4.0/rustdesk-1.4.0-x86_64.dmg", + ) + .expect("valid GitHub release asset URL"); + + assert_eq!( + file.file_name().and_then(|name| name.to_str()), + Some("rustdesk-1.4.0-x86_64.dmg") + ); + } + + #[test] + fn update_download_file_rejects_untrusted_or_malformed_urls() { + for url in [ + "http://github.com/rustdesk/rustdesk/releases/download/1/rustdesk.exe", + "https://example.com/rustdesk.exe", + "https://github.com/other/project/releases/download/1/rustdesk.exe", + "https://github.com/rustdesk/rustdesk/releases/download/1/", + "https://github.com/rustdesk/rustdesk/releases/download/1/nested/rustdesk.exe", + "https://github.com/rustdesk/rustdesk/releases/download/1/C:rustdesk.exe", + "https://user@github.com/rustdesk/rustdesk/releases/download/1/rustdesk.exe", + "https://github.com:443/rustdesk/rustdesk/releases/download/1/rustdesk.exe", + "https://github.com/rustdesk/rustdesk/releases/download/1/rustdesk.exe?download=1", + "https://github.com/rustdesk/rustdesk/releases/download/1/rustdesk.exe#download", + "not a url", + ] { + assert!(get_download_file_from_url(url).is_none(), "{url}"); + } + } +}