Issue 346: a setting a verb may set is pasted raw into its file, so an untrusted file is an injection point
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.
This commit is contained in:
@@ -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 = "<any path>
|
||||
|
||||
renders as `backend = "xrender"; write-pid-path = "<any 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`.**
|
||||
@@ -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:<key>}` 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"; <command>; X="` ends the quoted string and adds a command, and `$(<command>)` 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.
|
||||
Reference in New Issue
Block a user