diff --git a/rust/app/src/github_view.rs b/rust/app/src/github_view.rs index 493e384..c14e843 100644 --- a/rust/app/src/github_view.rs +++ b/rust/app/src/github_view.rs @@ -268,7 +268,11 @@ impl GithubView { }); match action { Some(Action::Zip(r)) => self.download(s, &r), - Some(Action::Visit(url)) if !url.is_empty() => ui.ctx().open_url(egui::OpenUrl::new_tab(url)), + Some(Action::Visit(url)) if !url.is_empty() => { + if !widgets::open_web_url(ui.ctx(), &url) { + s.error("The repository link is not a web address"); + } + } Some(Action::Copy(url)) => { ui.ctx().copy_text(url); s.info("URL copied"); diff --git a/rust/app/src/settings.rs b/rust/app/src/settings.rs index ceba6ef..ab98921 100644 --- a/rust/app/src/settings.rs +++ b/rust/app/src/settings.rs @@ -559,8 +559,11 @@ impl Settings { }); }); } - if !info.release_url.is_empty() && widgets::button(ui, icon::ARROW_SQUARE_OUT, "View release").clicked() { - ui.ctx().open_url(egui::OpenUrl::new_tab(&info.release_url)); + if widgets::is_web_url(&info.release_url) + && widgets::button(ui, icon::ARROW_SQUARE_OUT, "View release").clicked() + && !widgets::open_web_url(ui.ctx(), &info.release_url) + { + s.error("The release link is not a web address"); } } else { ui.label(RichText::new("You are running the latest version.").color(t.success)); diff --git a/rust/app/src/widgets.rs b/rust/app/src/widgets.rs index 0748565..d2d2d20 100644 --- a/rust/app/src/widgets.rs +++ b/rust/app/src/widgets.rs @@ -125,6 +125,24 @@ pub fn group(n: u64) -> String { out } +/// Whether `url` is a plain web link. Links that arrive from the network (update manifests, +/// GitHub API responses) are only handed to the system opener when they are http(s), so a +/// hostile or spoofed server can't launch `file:`, `ssh:` or custom-scheme handlers. +pub fn is_web_url(url: &str) -> bool { + let u = url.trim(); + (u.len() > 8 && u[..8].eq_ignore_ascii_case("https://")) || (u.len() > 7 && u[..7].eq_ignore_ascii_case("http://")) +} + +/// Opens `url` in the browser if it is a web link; returns false (and opens nothing) otherwise. +pub fn open_web_url(ctx: &egui::Context, url: &str) -> bool { + if !is_web_url(url) { + eprintln!("refusing to open non-web URL {url:?}"); + return false; + } + ctx.open_url(egui::OpenUrl::new_tab(url.trim())); + true +} + pub fn human_size(n: u64) -> String { const U: [&str; 5] = ["B", "KiB", "MiB", "GiB", "TiB"]; let mut v = n as f64; @@ -142,4 +160,14 @@ mod tests { fn helpers() { assert_eq!(super::group(1234567), "1,234,567"); } + + #[test] + fn only_web_urls_are_opened() { + for ok in ["https://dixon.cx/downloads/nexus/", "http://localhost:3000/x", " HTTPS://Example.com "] { + assert!(super::is_web_url(ok), "{ok}"); + } + for bad in ["", "https://", "file:///etc/passwd", "ssh://evil", "javascript:alert(1)", "ms-msdt:/id x", "ftp://host/", "//host/path"] { + assert!(!super::is_web_url(bad), "{bad}"); + } + } } diff --git a/rust/core/src/clients/vnc.rs b/rust/core/src/clients/vnc.rs index 9233542..9567966 100644 --- a/rust/core/src/clients/vnc.rs +++ b/rust/core/src/clients/vnc.rs @@ -17,6 +17,10 @@ pub use super::irc::{Connector, ReadWrite}; use super::{CloseFn, ErrorFn}; const READ_POLL: Duration = Duration::from_millis(50); +/// Largest desktop accepted from the server (64 Mpx = 256 MiB of RGBA; an 8K display is 33 Mpx). +const MAX_FB_PIXELS: usize = 64 << 20; +/// Largest single compressed rectangle (ZRLE) read from the server. +const MAX_ZRLE_BYTES: usize = 64 << 20; pub type FrameFn = Arc; pub type ResizeFn = Arc; @@ -80,6 +84,14 @@ impl Framebuffer { Framebuffer { width, height, rgba, dirty: true } } + /// A framebuffer of a server-announced size, refusing sizes that would exhaust memory. + fn new_checked(width: u16, height: u16) -> crate::Result { + if width == 0 || height == 0 || width as usize * height as usize > MAX_FB_PIXELS { + return Err(format!("unsupported desktop size {width}x{height}").into()); + } + Ok(Framebuffer::new(width, height)) + } + fn check(&self, x: u16, y: u16, w: u16, h: u16) -> crate::Result<()> { if x as usize + w as usize > self.width as usize || y as usize + h as usize > self.height as usize { return Err(format!("rectangle {w}x{h}+{x}+{y} outside the {}x{} framebuffer", self.width, self.height).into()); @@ -270,7 +282,7 @@ impl Client { fn run(&self, io: Box, out_rx: Receiver>) -> crate::Result<()> { let mut r = BufReader::with_capacity(1 << 16, Conn { io, out_rx }); let (w, h) = self.handshake(&mut r)?; - let mut local = Framebuffer::new(w, h); + let mut local = Framebuffer::new_checked(w, h)?; *self.fb.lock().unwrap_or_else(|e| e.into_inner()) = local.clone(); (self.cfg.on_resize)(w, h); { @@ -545,7 +557,7 @@ fn read_update(r: &mut impl Read, fb: &mut Framebuffer, dec: &mut Decoders) -> c } // ExtendedDesktopSize y is the status; non-zero answers a SetDesktopSize we never send. if enc == -223 || y == 0 { - *fb = Framebuffer::new(w, ht); + *fb = Framebuffer::new_checked(w, ht)?; resized = true; } continue; @@ -575,6 +587,9 @@ fn read_update(r: &mut impl Read, fb: &mut Framebuffer, dec: &mut Decoders) -> c 7 => tight(r, fb, dec, x, y, w, ht)?, 16 => { let len = read_u32(r)? as usize; + if len > MAX_ZRLE_BYTES { + return Err(format!("ZRLE rectangle of {len} bytes exceeds {MAX_ZRLE_BYTES}").into()); + } let data = read_n(r, len)?; inflate(&mut dec.zrle, &data, &mut dec.buf)?; zrle(&dec.buf, fb, x, y, w, ht)?; @@ -1038,4 +1053,22 @@ mod tests { fn apple_only_security_is_explained() { assert!(unsupported_security(&[30, 33]).contains("Apple Remote Desktop")); } + + #[test] + fn server_sizes_and_rect_lengths_are_bounded() { + assert!(Framebuffer::new_checked(7680, 4320).is_ok()); + assert!(Framebuffer::new_checked(0, 10).is_err()); + assert!(Framebuffer::new_checked(u16::MAX, u16::MAX).is_err()); + // A DesktopSize pseudo-rect announcing a 65535x65535 desktop is refused before allocating. + let mut m = vec![0, 0, 1]; + rect(&mut m, 0, 0, u16::MAX, u16::MAX, -223); + let mut fb = Framebuffer::new(4, 4); + let mut dec = Decoders::default(); + assert!(read_update(&mut &m[..], &mut fb, &mut dec).unwrap_err().to_string().contains("unsupported desktop size")); + // A ZRLE rect claiming 4 GiB of data is refused before allocating. + let mut m = vec![0, 0, 1]; + rect(&mut m, 0, 0, 4, 4, 16); + m.extend_from_slice(&u32::MAX.to_be_bytes()); + assert!(read_update(&mut &m[..], &mut fb, &mut dec).unwrap_err().to_string().contains("exceeds")); + } }