mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-01 05:37:23 -06:00
Fix #3604: automation file access refuses an NTFS alternate data stream name on Windows
With write root ...\out, a save to ...\out\x.svg:evil was accepted: it stays inside the root, but writes a hidden stream no folder listing shows. A ':' in any name after the drive is now refused with the device names. The check is a plain name test, so it is covered on every platform; the check-then-open race (item 1 of the report) is a documented limitation and is not touched.
This commit is contained in:
committed by
bflatastic
parent
feb12cf312
commit
811ba8727b
@@ -213,8 +213,10 @@ fn resolve(path: &str) -> Result<PathBuf, String> {
|
||||
return Err("automation path rejected: path is empty".into());
|
||||
}
|
||||
// Checked as given: making `…\NUL` absolute gives the device path `\\.\NUL` on Windows.
|
||||
if cfg!(windows) && Path::new(path).components().any(|c| matches!(c, Component::Normal(n) if is_device_name(&n.to_string_lossy()))) {
|
||||
return Err(format!("automation path rejected: reserved device names are not allowed: {path}"));
|
||||
if cfg!(windows)
|
||||
&& let Some(why) = windows_name_refusal(path)
|
||||
{
|
||||
return Err(format!("automation path rejected: {why}: {path}"));
|
||||
}
|
||||
let absolute = std::path::absolute(path).map_err(|e| format!("automation path rejected: cannot resolve {path}: {e}"))?;
|
||||
// The names below the nearest existing folder, last first.
|
||||
@@ -280,6 +282,18 @@ pub(crate) fn is_device_name(component: &str) -> bool {
|
||||
matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CLOCK$" | "CONIN$" | "CONOUT$") || numbered("COM") || numbered("LPT")
|
||||
}
|
||||
|
||||
/// Why Windows would not take `path` as an ordinary file name, for the first of its components
|
||||
/// that is a device name, or holds a `:` (a drive letter's own colon belongs to the prefix, not to
|
||||
/// a name, so only a stream such as `x.svg:evil` has one). (Judged by name alone, so it can be
|
||||
/// tested on any platform; `resolve` applies it on Windows only.)
|
||||
fn windows_name_refusal(path: &str) -> Option<&'static str> {
|
||||
Path::new(path).components().find_map(|c| match c {
|
||||
Component::Normal(n) if is_device_name(&n.to_string_lossy()) => Some("reserved device names are not allowed"),
|
||||
Component::Normal(n) if n.to_string_lossy().contains(':') => Some("alternate data streams are not allowed"),
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -497,4 +511,29 @@ mod tests {
|
||||
assert!(!is_device_name(f), "{f}");
|
||||
}
|
||||
}
|
||||
|
||||
/// #3604: on Windows `x.svg:evil` writes a hidden NTFS alternate data stream of `x.svg`, which
|
||||
/// stays inside the root yet shows in no folder listing, so a `:` in any component after the
|
||||
/// drive is refused (a drive letter's own colon is not a component).
|
||||
#[test]
|
||||
fn alternate_data_streams_are_refused() {
|
||||
for p in ["x.svg:evil", "out/x.svg:evil", "out/x.svg:evil:$DATA", "out/sub:evil/x.svg", "out/x.svg::$DATA", "out/:evil"] {
|
||||
assert_eq!(windows_name_refusal(p), Some("alternate data streams are not allowed"), "{p}");
|
||||
}
|
||||
for p in ["x.svg", "out/x.svg", "out/a b.svg", "out/x.svg.evil", "out/NUL"] {
|
||||
assert_ne!(windows_name_refusal(p), Some("alternate data streams are not allowed"), "{p}");
|
||||
}
|
||||
assert_eq!(windows_name_refusal("out/NUL"), Some("reserved device names are not allowed"));
|
||||
assert_eq!(windows_name_refusal("out/x.svg"), None);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_refuses_a_stream_inside_a_write_root() {
|
||||
let (base, roots) = folders("streams");
|
||||
let e = roots.check(&s(base.join("out").join("x.svg:evil")), Access::Write).unwrap_err();
|
||||
assert!(e.contains("alternate data streams"), "{e}");
|
||||
assert_eq!(roots.check(&s(base.join("out/x.svg")), Access::Write), Ok(()));
|
||||
let _ = std::fs::remove_dir_all(base);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Control protocol
|
||||
|
||||
> **Last updated:** 2026-10-25 · **Change:** Object → Expand… dimmed while the selection has an appearance Expand Appearance expands (`object.expand.info`'s `appearance`, #3089); previously: brush library panels (`window.brushLibrary`, `window.brushLibrary.other`, `ui.saveBrushLibrary`); previously: Transform Each opens on the values it was last confirmed with; its Copy button confirms with `copy: true` (#1732); previously: panel docking on craft-ui's layout: `ui.docking`, the parameters of `window.panel.float` and `window.panel.dock`; previously: `ui.screenshot`'s 64 MiB inline-PNG cap and its path-plus-data downgrade; symbol library panels (`window.symbolLibrary`, `window.symbolLibrary.other`, `ui.saveSymbolLibrary`); synthetic input budgets (#1709); bounded transport waits (#1726).
|
||||
> **Last updated:** 2026-10-31 · **Change:** Confining file access refuses a `:` in a name after the drive on Windows, an NTFS alternate data stream such as `x.svg:evil` (#3604); previously: Object → Expand… dimmed while the selection has an appearance Expand Appearance expands (`object.expand.info`'s `appearance`, #3089); previously: brush library panels (`window.brushLibrary`, `window.brushLibrary.other`, `ui.saveBrushLibrary`); previously: Transform Each opens on the values it was last confirmed with; its Copy button confirms with `copy: true` (#1732); previously: panel docking on craft-ui's layout: `ui.docking`, the parameters of `window.panel.float` and `window.panel.dock`; previously: `ui.screenshot`'s 64 MiB inline-PNG cap and its path-plus-data downgrade; symbol library panels (`window.symbolLibrary`, `window.symbolLibrary.other`, `ui.saveSymbolLibrary`); synthetic input budgets (#1709); bounded transport waits (#1726).
|
||||
|
||||
`vectorcraft --control <port>` listens on `127.0.0.1:<port>` (loopback only). The port must be a number
|
||||
from 1 to 65535 (`--control=<port>` works too), given once; otherwise the app prints an error and exits
|
||||
@@ -68,7 +68,7 @@ writes (save, export, package, captures, library saves…) inside a write root;
|
||||
ordinary error reply. Paths are made absolute against the app's working directory, links are followed
|
||||
(for a file that doesn't exist yet, its folder's), and the comparison is by whole folder names (`/work2` is
|
||||
not inside `/work`), ignoring case on Windows; `..` below a folder that doesn't exist, a link that leads
|
||||
nowhere and Windows device names are refused. A root left out grants none of its access. The roots must be
|
||||
nowhere, Windows device names and, on Windows, a `:` in any name after the drive (an NTFS alternate data stream such as `x.svg:evil`) are refused. A root left out grants none of its access. The roots must be
|
||||
existing folders, or the app doesn't start; without `--control` they are ignored. See `docs/mcp.md`
|
||||
› Confining file access for the details.
|
||||
|
||||
@@ -955,6 +955,7 @@ hidden tab (whose frames the browser pauses) answers when it is shown again.
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-31 | minor | Confining file access: on Windows a `:` in a name after the drive (an NTFS alternate data stream) is refused with the device names (#3604). |
|
||||
| 2026-10-25 | minor | Panel docking on craft-ui's layout: `ui.docking`, the floating and dock parameters, native floating windows. |
|
||||
| 2026-10-24 | minor | Documented the brush library panel commands (`window.brushLibrary`, `.other`, `ui.saveBrushLibrary`, `window.userBrushLibrary1`…`10`). |
|
||||
| 2026-10-24 | minor | Documented `ui.screenshot`'s 64 MiB inline-PNG reply cap: with `path` the file is still written and `pngBase64` is omitted when over the cap; a path-less `data: true` request fails before encoding. |
|
||||
|
||||
+2
-1
@@ -125,7 +125,8 @@ the suite) keep the agent to the folders you give it:
|
||||
doesn't exist is refused, as is a link that leads nowhere (writing through it would create its target).
|
||||
The result is compared with the root by whole folder names: `/work2/x` is not inside `/work`. On
|
||||
Windows case is ignored, UNC paths (`\\server\share\…`) are resolved like drive-letter ones, and
|
||||
device names (`CON`, `NUL`, `COM1`…) are refused.
|
||||
device names (`CON`, `NUL`, `COM1`…) and a `:` in a name after the drive (an NTFS alternate data stream
|
||||
such as `x.svg:evil`, which hides data in a stream no folder listing shows) are refused.
|
||||
- With `--connect`, the server makes the paths of `open_file`, `save_file`, `export` (`path` or
|
||||
`options.path`) and `screenshot {path}` absolute against its own working directory before it sends
|
||||
them to the app. Paths inside `run_command` and `command_batch` params go to the app as given, and the
|
||||
|
||||
Reference in New Issue
Block a user