libglacier-ng/SECURITY_REVIEW_2026-10-06.md

# Security review, 2026-10-06

Scope: every active source file in libglacier-ng (`src/*.c`, `src/*.h`,
not the vendored Monocypher, not the unbuilt early prototypes) and
glacier-ng (`gpkg`, `syspkg`, `gstore`, `glacier-bootstrap`, `gbuild`,
`gworld`, `lib/*.lua`, `tests/mkpkg`, the Dockerfiles). Read line by line;
findings confirmed by running them in Docker where possible; each fix
has a regression test, mutation-checked unless noted.

Tests: `glacier-ng/tests/audit_alpine.sh` (shell, `glacier-alpine_root`)
and `glacier-ng/tests/test_abort_unlocked.lua` (FFI suite).

## Findings and fixes

| # | Severity | Where | Finding | Fix | Test |
|---|---|---|---|---|---|
| 1 | **High** | `gl_install_pkg` | A package could ship top-level `files/.glacier-*` entries, glacier's own store files. As a symlink, `.glacier-fresh` made syspkg `open(O_CREAT\|O_TRUNC)` its target as root: an unsigned extra package truncated `/root/victim` to 0 bytes (reproduced). `.glacier-overwrite` granted the package `-O` without the user asking; `.glacier-manifest` rewrote its saved manifest. | Top-level names starting `.glacier-` are refused (`GL_MARKER_PREFIX`, common.h); the marker writes use `O_NOFOLLOW`. | audit_alpine: reserved names (mutants c3, c4 caught) |
| 2 | Medium | `gl_rmdir_recursive` (common.h) | `lstat` then `opendir` by path: root deleting a tree a user can write to (`gstore -x UID`) could be steered, by swapping a directory for a symlink between the two calls, into emptying the symlink's target. | fd-relative walk: `openat(O_DIRECTORY\|O_NOFOLLOW)`, `fdopendir`, `unlinkat`; the top path must not be a symlink. | audit_alpine: `gstore -x` race, 40 runs (mutant c1 caught 34-38/40) |
| 3 | Medium | `add_to_archive` (istoreutils.c) | Same class: `lstat` then `open`/`opendir` by path, no `O_NOFOLLOW`. Root backing up a user's store (`gstore -b`) could be steered into archiving root-only files. | fd-relative (`fstatat`/`openat(O_NOFOLLOW\|O_NONBLOCK)`/`readlinkat`); the opened descriptor's type is what gets archived. | **No failing-on-old test**: the window is too narrow for a shell race to hit (mutant c2 survived). Verified by review only. |
| 4 | Medium | `gl_gpm2gpkg` (pkg.c) | Manifests come from untrusted archives; libconfig honours `@include "FILE"`, so a manifest could make syspkg read any file as root. | A manifest with a line starting `@include` is refused (read into memory first, 1 MiB cap, no NUL). | audit_alpine (mutant c5 caught) |
| 5 | Low | `gl_gpm2gpkg` | A dependency/conflict entry without `PKG_NAME` ended the list early (later entries silently ignored, and leaked); names were not checked. | Every entry needs a `gl_valid_pkg_ident` name, else the manifest is refused. | audit_alpine (mutants c6, c7 caught) |
| 6 | Low | `gpkg_common.lua` | `read_manifest_fields` pattern-matched the manifest (via `tar` as root) and could disagree with the library: a decoy `// PKG_REPO = "community";` line hid that a package replaced an installed one, skipping the `-O` rule and the KEEP_OLD backup (two versions in the store). The tar extraction itself was not exploitable (GNU and busybox tar strip `..`; checked). | Replaced by `manifest_of`, a wrapper around `gl_read_pkg_manifest`; an unreadable manifest is refused before the prompt. | audit_alpine (old file as mutant l1, caught) |
| 7 | Low | `gstore`, `gpkg` | Text concatenated into `lg_printf` format strings: gstore's `argv[0]` and UID (as root), gpkg `-u`'s operand. | Passed as `%s` arguments; the `-u`/`-f`/`-s` stubs now refuse in one line. | audit_alpine (mutants l2, l3 caught) |
| 8 | Low | `gstore` | A UID above uid_t's range wrapped around in C to another UID. | Refused (`0..4294967294`). | audit_alpine (mutant l4 caught) |
| 9 | Low | `gl_abort_transaction` | Wiped the stage even for a context that never got the lock, i.e. another process's running transaction. | Only the lock holder's abort removes the stage (`GL_TXN_ERR_NO_TXN` otherwise). | test_abort_unlocked.lua (mutant f8 caught) |
| 10 | Low | `gpkg_common.lua` | Lua numbers passed to `lg_printf("%d")` (a double into varargs: undefined behaviour). | `%s` + `tostring`. | none (not observable) |
| 11 | Low | `gl_parse_semver` | `sscanf("%d")` on an untrusted, oversized number is undefined behaviour. | `strtol`, clamped to `0..INT_MAX`. | none (not observable) |
| 12 | Hardening | `is_glacier_symlink` | A plain prefix match: `/glacier/sys/store` also matched `/glacier/sys/store2/...` (only root can create such paths). | The prefix must be followed by `/` or end the target. | none |
| 13 | Bug | `join_with_cwd` (pkg.c) | `strlen(cwd + strlen(path) + 2)`: an out-of-bounds read. Unused. | Removed (see below). | n/a |
| 14 | Hardening | `lg_printf` | No format checking for C callers. | `__attribute__((format(printf, 2, 3)))`; the build is clean under `-Wformat=2 -Werror`. | the build |

Reviewed and found sound: signature code (`sign.c`: strict signify
parsing, secrets wiped, `O_NOFOLLOW|O_EXCL` writes), the index parser,
extraction's tar-slip defences (`is_safe_rel_path`, deferred symlinks,
`crosses_symlink`, `O_NOFOLLOW`), the sysroot trust check, syspkg's and
gstore's hardened loading, the private-copy verification, shell quoting
in every `os.execute`/`io.popen`, `/etc` config handling, `-O` displace
and restore.

## Removed as unused or redundant

- libglacier-ng: the `dag`, `runtime` and `make_conf` modules (nothing
  linked them); `gl_pkgver`, `gl_mkdir`, `gl_spawn_pkg`, `gl_cat_dir`,
  `gl_cat_dir_and_ver`, `gl_return_dir`, `gl_return_dir_and_ver`,
  `gl_free_path`, `gl_free_dir`, `gl_show_pkg_props`, `join_with_cwd`,
  `empty`/`null_dep`/`null_conf`, `dir_t`; `gl_parse_index`,
  `gl_free_index` (and `struct gindex`), `gl_find_pkg_repo` (duplicate of
  `gl_find_pkg_repos`), `gl_sys_istore_exists`, `gl_restore_istore`
  (also unsafe: followed symlinks when restoring); `lg_printf_no_newline`,
  `lg_syslog`, `lg_log` (its declaration and definition disagreed),
  `p2s`; `GL_CONF_PATH`; the broken `install_test_dir` Makefile target.
  The marker names, defined twice (deps.c, istoreutils.c), now live once
  in common.h; log.c includes log.h instead of redefining its enum.
- glacier-ng: `M.read_manifest_fields`, `M.extract_pkg_info`,
  `M.summary`; `install_with_reinstall` is module-local; the matching
  `glacier_cdef.lua` declarations.

## Open

- Finding 3 has no test that fails on the old code (see the table).
- Extracted entries keep setuid/setgid bits (known gap, unchanged).
- No size limits on extraction (a decompression bomb fills the disk).