Sicherheitsrunde: Befehlsinjektion, Pool-Pfad, Namensprüfung, Bild-Beacon
Vier Befunde aus einer Prüfung entlang der Angriffsflächen (MCP-Tools, Command-Grenze, Credentials, Schreibpfade und Abhängigkeiten): - todo.rs: Der SessionStart-Hook interpolierte den Projektpfad unquotiert in eine Shell-Kommandozeile, die Claude Code bei jedem Sessionstart ausfuehrt. Ein Ordner `repo$(...)` -- etwa aus einem geklonten Fremd-Repo -- ergab dauerhafte Codeausfuehrung; ein Leerzeichen zerbrach den Hook still. Der Pfad wird jetzt gequotet, inklusive Apostroph. - project_pool_dir: Der Pool-Name kommt aus der ai-control.json im Projektordner, also aus einer versionierten Datei. Ungeprueft bestimmte er mit ../ ein beliebiges Verzeichnis als CLAUDE_CONFIG_DIR, dessen settings.json wiederum apiKeyHelper traegt -- ein Kommando, das beim Sessionstart laeuft. Jetzt durch check_name abgesichert. - check_name: Steuerzeichen, Backslash und fuehrender Punkt werden abgelehnt. Ein Newline im Projektnamen hing bisher eigene Schluessel an die .desktop- Datei; Exec= braucht keinen Schraegstrich, den der alte Check verbot. - markdown.ts: Bilder duerfen nur noch lokale Ziele laden. Ein auswaertiges <img> laedt ohne Zutun und meldet damit IP, Zeitpunkt und im Pfad kodierte Daten -- der Panelinhalt stammt aus einer LLM-Session, ist also injizierbar. Schema-relative URLs (//host/x) waren durch ein zu laxes Muster erfasst. Links bleiben auswaerts erlaubt, jetzt mit rel="noopener noreferrer". escapeHtml escaped zusaetzlich das Apostroph. Dazu cargo audit nachgeholt: plist 1.9.0 -> 1.10.0 hebt quick-xml auf 0.41.0 und schliesst RUSTSEC-2026-0194/0195 fuer den Laufzeitpfad. Die zweite Instanz von quick-xml 0.39.4 bleibt ueber wayland-scanner im Lockfile -- ein proc-macro, das nur zur Compile-Zeit laeuft und ausgelieferte Protokoll-XMLs parst, nicht im Binary landet. 77 Rust-Tests, 39 Frontend-Tests.
This commit is contained in:
Generated
+13
-4
@@ -2988,13 +2988,13 @@ checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e"
|
||||
|
||||
[[package]]
|
||||
name = "plist"
|
||||
version = "1.9.0"
|
||||
version = "1.10.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "092791278e026273c1b65bbdcfbba3a300f2994c896bd01ab01da613c29c46f1"
|
||||
checksum = "7da1d65da6dd5d1e44199ac0f58712d241c0f439f80adea8924d832384087f85"
|
||||
dependencies = [
|
||||
"base64 0.22.1",
|
||||
"indexmap 2.14.0",
|
||||
"quick-xml",
|
||||
"quick-xml 0.41.0",
|
||||
"serde",
|
||||
"time",
|
||||
]
|
||||
@@ -3193,6 +3193,15 @@ dependencies = [
|
||||
"memchr",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "quick-xml"
|
||||
version = "0.41.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e660451e55124f798a69a5af3f49ccfbefbd41910eefd25caf2393e1f3473ec1"
|
||||
dependencies = [
|
||||
"memchr",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "quote"
|
||||
version = "1.0.46"
|
||||
@@ -5203,7 +5212,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "9c324a910fd86ebdc364a3e61ec1f11737d3b1d6c273c0239ee8ff4bc0d24b4a"
|
||||
dependencies = [
|
||||
"proc-macro2",
|
||||
"quick-xml",
|
||||
"quick-xml 0.39.4",
|
||||
"quote",
|
||||
]
|
||||
|
||||
|
||||
@@ -18,10 +18,49 @@ pub(crate) mod watcher;
|
||||
#[cfg(test)]
|
||||
pub(crate) mod testutil;
|
||||
|
||||
/// Namensprüfung für Projekte und Pool-Anzeigenamen (keine Pfad-Bestandteile).
|
||||
/// Namensprüfung für Projekte und Pool-Anzeigenamen.
|
||||
///
|
||||
/// Der Name wird als Pfadsegment verwendet (Pool-Verzeichnis, Panel-Dateien)
|
||||
/// und landet zugleich in Dateien mit zeilenbasiertem Format — vor allem in der
|
||||
/// `.desktop`-Datei unter Linux. Darum reicht das Verbot von `/` und `..`
|
||||
/// nicht: Ein Zeilenumbruch im Namen hängt dort eigene Schlüssel an, und
|
||||
/// `Exec=` braucht keinen Schrägstrich. Projektnamen stammen aus
|
||||
/// `dir.file_name()`, bei einem geklonten Fremd-Repo also von außen.
|
||||
pub(crate) fn check_name(name: &str) -> Result<(), String> {
|
||||
if name.trim().is_empty() || name.contains('/') || name.contains("..") {
|
||||
let ungueltig = name.trim().is_empty()
|
||||
|| name.contains('/')
|
||||
|| name.contains('\\')
|
||||
|| name.contains("..")
|
||||
|| name.starts_with('.')
|
||||
|| name.chars().any(|c| c.is_control());
|
||||
if ungueltig {
|
||||
return Err(format!("ungültiger Name: {name}"));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::check_name;
|
||||
|
||||
#[test]
|
||||
fn namen_pruefung() {
|
||||
for ok in ["projekt", "mein-projekt", "Projekt 2", "äöü"] {
|
||||
assert!(check_name(ok).is_ok(), "{ok} sollte gültig sein");
|
||||
}
|
||||
for bad in [
|
||||
"",
|
||||
" ",
|
||||
"a/b",
|
||||
"a\\b",
|
||||
"..",
|
||||
"../x",
|
||||
".versteckt",
|
||||
"boo\nExec=bash -c pwn", // .desktop-Injektion
|
||||
"a\tb",
|
||||
"a\u{7f}b",
|
||||
] {
|
||||
assert!(check_name(bad).is_err(), "{bad:?} sollte abgelehnt werden");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -156,9 +156,20 @@ pub(crate) fn list_projects_in(paths: &Paths) -> Result<Vec<Project>, String> {
|
||||
|
||||
/// Pool-Config-Verzeichnis eines Projekts — wird dem Terminal als
|
||||
/// CLAUDE_CONFIG_DIR mitgegeben.
|
||||
///
|
||||
/// Der Name wird geprüft, obwohl `assign_pool_in` das beim Zuweisen schon tut:
|
||||
/// Die Quelle ist die `ai-control.json` **im Projektordner**, also eine
|
||||
/// versionierte Datei, die mit einem geklonten Repo hereinkommt. Ungeprüft
|
||||
/// bestimmte sie mit `../…` ein beliebiges Verzeichnis als CLAUDE_CONFIG_DIR —
|
||||
/// und dessen `settings.json` trägt `apiKeyHelper`, ein Kommando, das beim
|
||||
/// Sessionstart ausgeführt wird.
|
||||
pub(crate) fn project_pool_dir(project: &str) -> Result<Option<PathBuf>, String> {
|
||||
let paths = Paths::real();
|
||||
Ok(read_project_config_in(&paths, project)?.pool.map(|p| paths.pool_dir(&p)))
|
||||
let Some(pool) = read_project_config_in(&paths, project)?.pool else {
|
||||
return Ok(None);
|
||||
};
|
||||
check_name(&pool).map_err(|_| format!("ungültiger Pool in {PROJECT_FILE}: {pool}"))?;
|
||||
Ok(Some(paths.pool_dir(&pool)))
|
||||
}
|
||||
|
||||
/// Terminal-Einstellungen eines Projekts, für den Terminal-Prozess.
|
||||
@@ -534,6 +545,21 @@ mod tests {
|
||||
assert_eq!(v["pool"], serde_json::Value::String(pool));
|
||||
}
|
||||
|
||||
/// Ein Pool-Eintrag aus einer mitgeklonten `ai-control.json` darf kein
|
||||
/// beliebiges Verzeichnis zum CLAUDE_CONFIG_DIR machen.
|
||||
#[test]
|
||||
fn pool_aus_projektdatei_mit_traversal_wird_abgelehnt() {
|
||||
let p = tmp_paths();
|
||||
create_project(&p, "fremd").unwrap();
|
||||
let cfg_path = project_config_path(&p, "fremd").unwrap();
|
||||
fs::write(&cfg_path, r#"{"pool":"../../../../tmp/x"}"#).unwrap();
|
||||
|
||||
let cfg = read_project_config_in(&p, "fremd").unwrap();
|
||||
assert_eq!(cfg.pool.as_deref(), Some("../../../../tmp/x"));
|
||||
// Gelesen wird der Wert noch, aber er darf keinen Pfad bilden.
|
||||
assert!(check_name(cfg.pool.as_deref().unwrap()).is_err());
|
||||
}
|
||||
|
||||
/// Das Setzen der Terminal-Config kommt aus der Oberfläche und kennt nur
|
||||
/// theme/icon/title — unbekannte Keys müssen trotzdem stehen bleiben.
|
||||
#[test]
|
||||
|
||||
@@ -11,13 +11,26 @@ use crate::domain::registry::project_dir;
|
||||
pub(crate) const TODO_FILE: &str = "OFFENE-PUNKTE.md";
|
||||
const TODO_SKELETON: &str = "# Offene Punkte — bei jedem Start prüfen und abhaken\n\nKeine offenen Punkte.\n";
|
||||
|
||||
/// Der Hook landet in der `settings.json` des Projekts und wird von Claude Code
|
||||
/// bei jedem Sessionstart über die Shell ausgeführt. Der Pfad muss darum
|
||||
/// gequotet werden: Er stammt aus dem Ordnernamen, den der Nutzer im Dialog
|
||||
/// wählt — bei einem geklonten Fremd-Repo also von außen. Unquotiert genügte
|
||||
/// ein Ordner `repo$(…)` für dauerhafte Codeausführung, und schon ein
|
||||
/// Leerzeichen im Pfad hätte den Hook still zerbrochen.
|
||||
fn todo_hook_command(dir: &std::path::Path) -> String {
|
||||
format!(
|
||||
"jq -Rs '{{systemMessage: ., hookSpecificOutput:{{hookEventName:\"SessionStart\", additionalContext: .}}}}' {}",
|
||||
dir.join(TODO_FILE).display()
|
||||
shell_quote(&dir.join(TODO_FILE).to_string_lossy())
|
||||
)
|
||||
}
|
||||
|
||||
/// Ein Argument für `sh -c` in einfache Anführungszeichen setzen. Innerhalb
|
||||
/// davon ist jedes Zeichen literal; einzig das Apostroph selbst muss die
|
||||
/// Quotierung verlassen und wieder betreten (`'\''`).
|
||||
fn shell_quote(s: &str) -> String {
|
||||
format!("'{}'", s.replace('\'', r"'\''"))
|
||||
}
|
||||
|
||||
fn hook_is_todo(group: &serde_json::Value) -> bool {
|
||||
group["hooks"]
|
||||
.as_array()
|
||||
@@ -98,6 +111,21 @@ mod tests {
|
||||
use crate::domain::project::{create_project_full_in, TerminalConfig};
|
||||
use crate::domain::testutil::{create_project, tmp_paths};
|
||||
|
||||
/// Der Hook-Befehl geht durch die Shell; ein Pfad mit Metazeichen darf dort
|
||||
/// nichts ausführen. Ordnernamen sind bei geklonten Repos Fremdeingabe.
|
||||
#[test]
|
||||
fn hook_befehl_quotet_den_pfad() {
|
||||
let cmd = todo_hook_command(std::path::Path::new("/tmp/repo$(touch /tmp/pwned)"));
|
||||
assert!(cmd.ends_with("'/tmp/repo$(touch /tmp/pwned)/OFFENE-PUNKTE.md'"), "{cmd}");
|
||||
|
||||
// Ein Apostroph im Pfad darf die Quotierung nicht aufbrechen.
|
||||
let cmd = todo_hook_command(std::path::Path::new("/tmp/o'brien"));
|
||||
assert!(cmd.ends_with(r"'/tmp/o'\''brien/OFFENE-PUNKTE.md'"), "{cmd}");
|
||||
// Nach dem Zerlegen an den Quotes bleibt kein unquotierter Bereich übrig,
|
||||
// in dem eine Shell noch etwas zu interpretieren hätte.
|
||||
assert!(!cmd.contains("$("));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn todo_zuschalten_und_abschalten() {
|
||||
let p = tmp_paths();
|
||||
|
||||
+37
-3
@@ -39,10 +39,44 @@ describe("renderMarkdown", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("blockt data:-Bilder, lässt normale Ziele durch", () => {
|
||||
it("blockt data:-Bilder", () => {
|
||||
expect(renderMarkdown(">)")).not.toContain("<img");
|
||||
expect(renderMarkdown("")).toContain("<img");
|
||||
expect(renderMarkdown("[a](https://example.org)")).toContain('href="https://example.org"');
|
||||
});
|
||||
|
||||
/// Ein <img> lädt ohne Zutun. Auswärtige Bildquellen sind darum ein
|
||||
/// Zero-Click-Beacon: IP, Zeitpunkt und im Pfad kodierte Daten gehen raus,
|
||||
/// sobald jemand ein vergiftetes Archiv-Dokument nur ansieht.
|
||||
it("lädt keine auswärtigen Bilder, auch nicht schema-relativ", () => {
|
||||
for (const src of [
|
||||
"https://evil.example/t.png",
|
||||
"http://evil.example/t.png",
|
||||
"//evil.example/t.png",
|
||||
"HTTPS://evil.example/t.png",
|
||||
]) {
|
||||
const el = document.createElement("div");
|
||||
el.innerHTML = renderMarkdown(``);
|
||||
expect(el.querySelector("img"), `${src} durfte kein img erzeugen`).toBeNull();
|
||||
expect(el.textContent).toContain("alt");
|
||||
}
|
||||
});
|
||||
|
||||
it("lässt lokale Bilder durch", () => {
|
||||
expect(renderMarkdown("")).toContain("<img");
|
||||
expect(renderMarkdown("")).toContain("<img");
|
||||
});
|
||||
|
||||
/// Links dürfen auswärts zeigen — sie brauchen einen Klick.
|
||||
it("erlaubt auswärtige Links, aber ohne Referrer und Opener", () => {
|
||||
const el = document.createElement("div");
|
||||
el.innerHTML = renderMarkdown("[a](https://example.org)");
|
||||
const a = el.querySelector("a")!;
|
||||
expect(a.getAttribute("href")).toBe("https://example.org");
|
||||
expect(a.getAttribute("rel")).toContain("noopener");
|
||||
expect(a.getAttribute("rel")).toContain("noreferrer");
|
||||
});
|
||||
|
||||
it("escapt auch das Apostroph", () => {
|
||||
expect(renderMarkdown("Text mit ' Apostroph")).toContain("'");
|
||||
});
|
||||
|
||||
it("escapt Anführungszeichen im title, damit das Attribut nicht aufbricht", () => {
|
||||
|
||||
+31
-11
@@ -13,25 +13,41 @@
|
||||
|
||||
import { marked, type Tokens } from "marked";
|
||||
|
||||
/// Schemata, die im Panel etwas anzuzeigen haben. Alles andere — allen voran
|
||||
/// `javascript:`, aber auch `data:` (SVG mit Skript) und `file:` — fliegt raus.
|
||||
const OK_SCHEME = /^(https?:|mailto:|#|\/|\.{0,2}\/)/i;
|
||||
/// Ziele ohne Schema: Anker, absolute und relative Pfade innerhalb der App.
|
||||
/// Beide Schrägstrich-Formen müssen den doppelten ausschließen — `//host/x` ist
|
||||
/// eine schema-relative URL und damit auswärtig. Darum `\/(?!\/)` für den
|
||||
/// absoluten Pfad und `\.{1,2}\/` für den relativen: Ein `{0,2}` würde den
|
||||
/// nackten `/` mitmatchen und die erste Regel wirkungslos machen.
|
||||
const LOCAL = /^(#|\/(?!\/)|\.{1,2}\/)/;
|
||||
|
||||
/// Links dürfen auswärts zeigen: Sie brauchen einen Klick, und ein Archiv-
|
||||
/// Dokument ohne funktionierende Quellenangaben wäre nutzlos.
|
||||
const OK_LINK = new RegExp(`^(https?:|mailto:)|${LOCAL.source}`, "i");
|
||||
|
||||
/// Bilder dagegen nur lokal. Ein `<img>` lädt beim bloßen Anzeigen, ohne jedes
|
||||
/// Zutun: Ein vergiftetes Archiv-Dokument — der Panel-Inhalt kommt aus einer
|
||||
/// LLM-Session und ist damit prompt-injizierbar — meldet über die Bild-URL
|
||||
/// still IP, Zeitpunkt und im Pfad kodierte Daten nach außen.
|
||||
const OK_IMAGE = LOCAL;
|
||||
|
||||
function escapeHtml(s: string): string {
|
||||
return s
|
||||
.replace(/&/g, "&")
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, """);
|
||||
.replace(/"/g, """)
|
||||
// Auch das Apostroph: Sonst hängt die Sicherheit daran, dass jedes Attribut
|
||||
// hier doppelt gequotet bleibt — eine Konvention, keine Garantie.
|
||||
.replace(/'/g, "'");
|
||||
}
|
||||
|
||||
/// Leerer Link statt gefährlichem Ziel — der Text bleibt sichtbar, der Klick
|
||||
/// tut nichts.
|
||||
function safeHref(href: string): string {
|
||||
/// Leeres Ziel statt gefährlichem — der Text bleibt sichtbar, der Klick tut
|
||||
/// nichts.
|
||||
function safeHref(href: string, erlaubt: RegExp): string {
|
||||
const h = href.trim();
|
||||
// Steuerzeichen entfernen: `java\nscript:` ist sonst ein Umgehungsweg.
|
||||
const clean = h.replace(/[\u0000-\u001f\u007f]/g, "");
|
||||
return OK_SCHEME.test(clean) ? clean : "";
|
||||
return erlaubt.test(clean) ? clean : "";
|
||||
}
|
||||
|
||||
const renderer = new marked.Renderer();
|
||||
@@ -42,13 +58,17 @@ renderer.html = ({ raw }: Tokens.HTML | Tokens.Tag) => escapeHtml(raw);
|
||||
|
||||
renderer.link = function ({ href, title, tokens }: Tokens.Link) {
|
||||
const text = this.parser.parseInline(tokens);
|
||||
const safe = safeHref(href);
|
||||
const safe = safeHref(href, OK_LINK);
|
||||
const t = title ? ` title="${escapeHtml(title)}"` : "";
|
||||
return safe ? `<a href="${escapeHtml(safe)}"${t}>${text}</a>` : `<a${t}>${text}</a>`;
|
||||
// `rel` auch bei fehlendem target: Der Webview soll dem Ziel keinen Bezug auf
|
||||
// das öffnende Fenster und keinen Referrer mitgeben.
|
||||
const rel = ` rel="noopener noreferrer"`;
|
||||
return safe ? `<a href="${escapeHtml(safe)}"${t}${rel}>${text}</a>` : `<a${t}>${text}</a>`;
|
||||
};
|
||||
|
||||
renderer.image = ({ href, title, text }: Tokens.Image) => {
|
||||
const safe = safeHref(href);
|
||||
const safe = safeHref(href, OK_IMAGE);
|
||||
// Auswärtiges Bild: nur der Alt-Text, keine Anfrage nach draußen.
|
||||
if (!safe) return escapeHtml(text);
|
||||
const t = title ? ` title="${escapeHtml(title)}"` : "";
|
||||
return `<img src="${escapeHtml(safe)}" alt="${escapeHtml(text)}"${t}>`;
|
||||
|
||||
Reference in New Issue
Block a user