From 79f31ca113f592fae8716ea297cc85f49647f186 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 12:35:30 +0200 Subject: [PATCH 1/2] Issue 346: a setting a verb may set is pasted raw into its file, so an untrusted file is an injection point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found in the review of the picom settings: a backend of xrender"; write-pid-path = "… adds an option of the caller's. The two files marked trusted false on main are judged (user-dirs.dirs open, dunstrc not), and typed settings validated at the verb and quoted at render are proposed; until then nothing new goes trusted false. --- .../00-report.md | 57 +++++++++++++++++++ .../01-diagnosis.md | 56 ++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md create mode 100644 04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md diff --git a/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md new file mode 100644 index 00000000..6ca4ff6f --- /dev/null +++ b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md @@ -0,0 +1,57 @@ +--- +status: located +opened: 2026-10-09 +located-in: [mesh-controller (internal/catalogue/setting_into.go settingInto, internal/catalogue/bound_into_files.go plainly, internal/catalogue/terminal_keys.go), mesh-catalog (every file a module marks "trusted": false)] +fixed-by: +amended-design: +--- + +# 346. A setting a verb may set is pasted raw into its file, so an untrusted file is an injection point + +**Severity: high.** Through a file whose format has a statement separator or quoting, any caller of the +settings verb can add a line of its own to a program's configuration. + +## Symptom + +A review on 2026-10-09 found this in a pull request (mesh-catalog #157, the `picom` module's `backend` and +`use-damage` settings, issue 345), before it merged. Nothing was run against a node. + +The pull request marked `picom.conf` `"trusted": false`, so that a machine's backend could be set through +the `settings` verb ([issue 339](../339-a-verb-can-place-a-directory-at-the-machines-own/00-report.md)). +The file says `backend = "${setting:backend}";`. A value of + + xrender"; write-pid-path = " + +renders as `backend = "xrender"; write-pid-path = "";`, and picom then overwrites any file the +operator's account can write. The same tail adds `log-file`, a shader path, a window rule that hides +windows, or `backend = "dummy"`, which blanks the screen. The unquoted `use-damage = ${setting:use-damage};` +takes the same tail without the quote. + +The pull request was changed to keep the file trusted (terminal-only). The defect is general. + +## Why it is an issue + +- The controller fills `${setting:…}` with the value **as it is** (`settingInto` → `plainly`): a string is + written verbatim. It refuses only a line break, a carriage return, another line end and a NUL + (`settingsHoldOneLine`, issue 339). +- A setting has a `kind` (`preference`) and a `default`, and **no type and no allowed values**. Nothing + says `backend` is one of `glx`, `xrender`, or that `use-damage` is a boolean. +- `"trusted": false` (issue 339) opens a file's settings to the `settings` verb. It was meant for "a file + nothing but its own person reads". But a file read only by the person's own program can still make that + program do anything its format allows. Whether a raw value is safe depends on the file's format, and + nothing judges the format. + +So the rule "trusted: false where nothing trusts it" is not enough. A one-line value is safe only where +the format has no way to end a statement or a quoted string within a line. + +## Impact + +- Any caller of the `settings` verb (an agent session, a tool) can rewrite a desktop program's + configuration beyond the key it was given, as the operator's account. +- Two files are marked untrusted on main today. The survey in [01-diagnosis.md](01-diagnosis.md) judges + both: the `xdg` module's `user-dirs.dirs` is open to the same injection, `dunst`'s `dunstrc` is not. + +## What is asked + +The fix is proposed in the diagnosis and needs a decision record (settings get a type), then a controller +change. Until it ships, **no new file is marked `"trusted": false`.** diff --git a/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md new file mode 100644 index 00000000..92fb0c17 --- /dev/null +++ b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md @@ -0,0 +1,56 @@ +# 346 — Diagnosis + +2026-10-09. Read from mesh-controller `4d60628` and mesh-catalog main `7339ce8`. Nothing was run against +a node. + +## Where a value goes + +1. The `settings` verb stores a layer of values per module and node. Before storing, and again when it + composes, `settingsHoldOneLine` refuses a line break, a carriage return, another line end or a NUL in any + string. Nothing else is judged. +2. `settingInto` (internal/catalogue/setting_into.go) replaces each `${setting:}` in a file's content + with `plainly(value)` (internal/catalogue/bound_into_files.go). For a string that is the string itself; a + number is printed without trailing zeros; a boolean as `true`/`false`. No quoting and no escaping, for + any format. +3. `TerminalKeys` (internal/catalogue/terminal_keys.go) makes every key a file asks for terminal-only, unless + the file says `"trusted": false`. Then the verb may set it. +4. A module's `settings` entry has `kind`, `default` and `why`. There is no type, no range and no list of + allowed values, so neither the verb nor the composition can refuse `xrender"; …` for a backend. + +## Every file marked "trusted": false on main + +| module | file | the line | format | verdict | +|---|---|---|---|---| +| `dunst` | `~/.config/dunst/dunstrc` | `width = ${setting:width}` and `font = Inter ${setting:font-size}` | INI: one key per line, the value to the end of the line, no separator or quoting within a line | **not open** to adding a key: without a line break a value stays the value of its own key. A wrong value (`width = abc`) makes dunst fall back or refuse the file: an outage of notifications, not an injection | +| `xdg` | `~/.config/user-dirs.dirs` | `XDG_DESKTOP_DIR="${setting:desktop-dir}"`, and the same for `publicshare-dir` and `videos-dir` | shell assignments; the xdg-user-dirs format is defined to be sourced by a shell | **open.** A value of `$HOME/x"; ; X="` ends the quoted string and adds a command, and `$()` inside the quotes runs too, in every shell or script that sources the file. The programs measured on the laptop parse it instead (`xdg-user-dir` is a C program there, and GLib parses), and no managed file sources it; a script of the operator's, or a tool that follows the format's definition, would run the command. Even parsed, a value can point the desktop or public folder anywhere the account can write | + +Mergeable JSON files ("any mergeable file takes any key", issue 340) are not pasted: values are merged as +JSON and encoded by the encoder. They are outside this defect. + +**Recommendation for now:** mark `user-dirs.dirs` trusted again (terminal-only) in a catalogue change, as +`picom.conf` was kept in mesh-catalog #157. `dunstrc` may stay as it is. + +## The fix proposed + +A decision record, then a controller change. The coordinator schedules it; it is not built here. + +1. **A setting declares a type** in the manifest: `bool`, `int` (with an optional range), `enum` (its + allowed values), or `string` with the **format it is written into**, for example `ini-value`, + `shell-double-quoted`, `libconfig-string`, `path`. A setting without a type is a string that only the + terminal may set. +2. **The controller validates at the verb**, before a layer is stored: a bool is a JSON boolean, an int an + integer in range, an enum one of its values, a string one line. A refusal names the key and the type. + Validated again at composition, for layers stored before. +3. **The controller quotes at render** with the format's own function: `shell-double-quoted` escapes + `"`, `\`, `$` and backquote; `libconfig-string` escapes `"` and `\`; `ini-value` refuses `;`, `#` and a + leading `[`, or quotes where the format allows. A bool, an int or an enum needs no quoting, because it + cannot hold one. +4. **`"trusted": false` requires every setting the file asks for to be typed**, checked by `module check` + in the catalogue's merge check, so an untyped string can never reach an untrusted file again. +5. `picom`'s `backend` becomes `enum [glx, xrender, egl]` and `use-damage` a `bool`; then `picom.conf` may + be untrusted. `xdg`'s three folders become `shell-double-quoted` paths under `$HOME/`. + +**Until then, no new file is marked `"trusted": false`.** How that rule is checked: `module check` already +counts the files that ask for a setting without saying whether they are trusted; the catalogue's merge +check can refuse any newly added `"trusted": false` (a list that only shrinks, like `health-undeclared`). +That check is part of the fix, not built here. From 0ff6bd2267267db71853baa9aa5a9b3c6002be51 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 12:40:26 +0200 Subject: [PATCH 2/2] Issue 346: user-dirs.dirs is trusted again for now (mesh-catalog #158), the stop-gap before the fix --- .../00-report.md | 3 ++- .../01-diagnosis.md | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md index 6ca4ff6f..d98e62f0 100644 --- a/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md +++ b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/00-report.md @@ -48,8 +48,9 @@ the format has no way to end a statement or a quoted string within a line. - Any caller of the `settings` verb (an agent session, a tool) can rewrite a desktop program's configuration beyond the key it was given, as the operator's account. -- Two files are marked untrusted on main today. The survey in [01-diagnosis.md](01-diagnosis.md) judges +- Two files were marked untrusted on main. The survey in [01-diagnosis.md](01-diagnosis.md) judges both: the `xdg` module's `user-dirs.dirs` is open to the same injection, `dunst`'s `dunstrc` is not. + `user-dirs.dirs` is made trusted again for now (mesh-catalog #158). ## What is asked diff --git a/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md index 92fb0c17..8cd64cd2 100644 --- a/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md +++ b/04-ISSUES/346-a-setting-a-verb-may-set-is-pasted-raw-into-its-file/01-diagnosis.md @@ -27,8 +27,10 @@ a node. Mergeable JSON files ("any mergeable file takes any key", issue 340) are not pasted: values are merged as JSON and encoded by the encoder. They are outside this defect. -**Recommendation for now:** mark `user-dirs.dirs` trusted again (terminal-only) in a catalogue change, as -`picom.conf` was kept in mesh-catalog #157. `dunstrc` may stay as it is. +**Done for now (2026-10-09):** `user-dirs.dirs` is marked trusted again (terminal-only) in mesh-catalog +#158, as `picom.conf` was kept in mesh-catalog #157. A test in each module fails if its file carries a +trusted marking, naming this issue. `dunstrc` stays as it is. This is the stop-gap, not the fix: the +fix below still stands. ## The fix proposed