Merge pull request #864 from dylanmtaylor/security/release-url-confinement

security: confine release HTML URLs and sanitize browser open links (F-004, F-007)

crates/core/src/feed.rs

@@ -206,7 +206,12 @@ impl FeedRelease {
             name: if self.name.is_empty() { self.tag.clone() } else { truncate(&self.name, 120) },
             tag: self.tag.clone(),
             published_at: self.published_at,
-            html_url: if self.html_url.starts_with(&releases_page) { self.html_url.clone() } else { format!("{releases_page}tag/{}", self.tag) },
+            html_url: release::safe_release_html_url(repo, &self.html_url)
+                .or_else(|| {
+                    let fallback = format!("{releases_page}tag/{}", self.tag);
+                    release::safe_release_html_url(repo, &fallback)
+                })
+                .unwrap_or_else(|| format!("https://github.com/{repo}/releases")),
             notes: truncate(&self.notes, 64 * 1024),
             prerelease: self.prerelease,
             assets,
@@ -275,7 +280,7 @@ struct RawApp {
 impl RawApp {
     fn into_app(self) -> FeedApp {
         FeedApp {
-            repo: self.repo.filter(|r| r.split('/').count() == 2),
+            repo: self.repo.filter(|r| r.split('/').count() == 2 && r.split('/').all(|part| !part.is_empty() && part != "." && part != "..")),
             release: self.latest_release.and_then(RawRelease::into_release),
             fetched_at: self.app_last_fetched.as_deref().and_then(parse_rfc3339),
         }
@@ -372,7 +377,7 @@ impl RawRepoRename {
     fn into_rename(self) -> Option<RepoRename> {
         let ok = |r: &str| {
             let mut parts = r.split('/');
-            matches!((parts.next(), parts.next(), parts.next()), (Some(a), Some(b), None) if a.eq_ignore_ascii_case(GITHUB_OWNER) && !b.is_empty()
+            matches!((parts.next(), parts.next(), parts.next()), (Some(a), Some(b), None) if a.eq_ignore_ascii_case(GITHUB_OWNER) && !b.is_empty() && b != "." && b != ".."
           && r.bytes().all(|c| c.is_ascii_alphanumeric() || b"-_./".contains(&c)))
         };
         (ok(&self.old_name) && ok(&self.new_name)).then(|| RepoRename {

crates/core/src/release.rs

@@ -85,6 +85,31 @@ pub fn is_shell_meta(c: char) -> bool {
     matches!(c, '|' | '&' | ';' | '<' | '>' | '(' | ')' | '$' | '`' | '"' | '\'' | '^' | '!' | '*' | '%' | '{' | '}' | '[' | ']')
 }
 
+/// Whether `url` is a valid release page URL strictly for `repo` on GitHub:
+/// starts with `https://github.com/<repo>/releases`, contains no dot-segments (`.` or `..`),
+/// no percent-encoded path separators (`%2f`, `%5c`), no query/fragment/backslash, and
+/// satisfies [`safe_web_link`] (no shell metacharacters or control bytes, CWE-78).
+pub fn safe_release_html_url(repo: &str, url: &str) -> Option<String> {
+    if !safe_web_link(url) {
+        return None;
+    }
+    let releases = format!("https://github.com/{repo}/releases");
+    if url == releases {
+        return Some(url.to_string());
+    }
+    let rest = url.strip_prefix(&releases)?;
+    let path = rest.strip_prefix('/')?;
+    if path.is_empty() || path.contains(['?', '#', '\\']) {
+        return None;
+    }
+    for seg in path.split('/') {
+        if seg.is_empty() || seg == "." || seg == ".." || seg.contains("%2f") || seg.contains("%2F") || seg.contains("%5c") || seg.contains("%5C") {
+            return None;
+        }
+    }
+    Some(url.to_string())
+}
+
 /// The prefix every download of `repo` must start with.
 pub fn download_prefix(repo: &str) -> String {
     format!("https://github.com/{repo}/releases/download/")
@@ -168,10 +193,7 @@ fn release_from_value(v: &serde_json::Value, repo: &str) -> Result<Release, Stri
     }
     let tag = v["tag_name"].as_str().filter(|t| !t.is_empty() && t.len() <= 64).ok_or("no release found")?.to_string();
     let releases = format!("https://github.com/{repo}/releases");
-    let html_url = v["html_url"]
-        .as_str()
-        .filter(|u| u.strip_prefix(&releases).is_some_and(|rest| rest.starts_with('/') && !rest.contains(['?', '#', '\\'])))
-        .map_or_else(|| releases.clone(), str::to_string);
+    let html_url = v["html_url"].as_str().and_then(|u| safe_release_html_url(repo, u)).unwrap_or_else(|| releases.clone());
     let prefix = download_prefix(repo);
     let assets = v["assets"]
         .as_array()
@@ -335,6 +357,31 @@ mod tests {
         assert!(parse_github_release("<html>", REPO).is_err());
     }
 
+    #[test]
+    fn release_html_url_rejects_traversal_and_metacharacters() {
+        let repo = "storytold/photocraft";
+        assert_eq!(
+            safe_release_html_url(repo, "https://github.com/storytold/photocraft/releases/tag/v1.0.0"),
+            Some("https://github.com/storytold/photocraft/releases/tag/v1.0.0".to_string())
+        );
+        assert_eq!(
+            safe_release_html_url(repo, "https://github.com/storytold/photocraft/releases"),
+            Some("https://github.com/storytold/photocraft/releases".to_string())
+        );
+        for bad in [
+            "https://github.com/storytold/photocraft/releases/../../attacker/repo/releases/tag/v1",
+            "https://github.com/storytold/photocraft/releases/tag/v1/../../../attacker/repo",
+            "https://github.com/storytold/photocraft/releases/tag/v1|calc.exe",
+            "https://github.com/storytold/photocraft/releases/tag/v1%2f..%2f..",
+            "https://github.com/storytold/photocraft/releases/tag/v1?x=1",
+            "https://github.com/storytold/photocraft/releases/tag/v1#hash",
+            "https://github.com/storytold/other/releases/tag/v1.0.0",
+            "http://github.com/storytold/photocraft/releases/tag/v1",
+        ] {
+            assert!(safe_release_html_url(repo, bad).is_none(), "should reject: {bad}");
+        }
+    }
+
     #[test]
     fn traversal_cannot_escape_the_repository() {
         // Raw-prefix matches but the interpreted path climbs into another repository (CWE-180).

crates/ui-egui/src/widgets.rs

@@ -280,7 +280,11 @@ pub fn hud_link(ui: &mut Ui, text: &str, url: &str) -> Response {
     resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Link, true, text));
     let resp = resp.on_hover_text(url).on_hover_cursor(egui::CursorIcon::PointingHand);
     if resp.clicked() {
-        ui.ctx().open_url(egui::OpenUrl::new_tab(url));
+        if artcraft_launcher_core::release::safe_web_link(url) {
+            ui.ctx().open_url(egui::OpenUrl::new_tab(url));
+        } else {
+            log::warn!("refusing to open unsafe URL: {url}");
+        }
     }
     resp
 }