diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml new file mode 100644 index 0000000..f1cb828 --- /dev/null +++ b/.gitea/workflows/ci.yml @@ -0,0 +1,69 @@ +# Gitea Actions. See docs/CI.md for what each job needs, the one-time runner setup, and the +# known gaps between Gitea Actions and GitHub Actions this file was written around (notably: +# timeout-minutes and concurrency are parsed but not enforced by Gitea as of the 1.25 docs, so +# the real cap on every step is the `timeout ` wrapped around it, not the YAML key). +name: ci + +on: + push: + branches: ["**"] + pull_request: + +jobs: + # Needs only a C++20 toolchain and CMake: runs on any Docker-mode act_runner. + build: + runs-on: ubuntu-latest + timeout-minutes: 10 # label only on Gitea (see docs/CI.md); the real cap is the `timeout` below + steps: + - uses: actions/checkout@v4 + - name: install build dependencies + run: timeout 180 sh -c 'apt-get update && apt-get install -y --no-install-recommends build-essential cmake' + - name: build (-Werror) + run: timeout 300 cmake -S . -B build && timeout 300 cmake --build build + - name: docs cross-link check + run: timeout 60 scripts/check-docs.sh + + # No special runner privilege needed: scripts/valgrind.sh only fails on what valgrind itself + # reports, never on doos's own exit code, so it is still a real check even when this container's + # own policy blocks the user namespace doos tries to create (see docs/CI.md). + valgrind: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + - name: install build and valgrind dependencies + run: timeout 180 sh -c 'apt-get update && apt-get install -y --no-install-recommends build-essential cmake valgrind' + - name: build + run: timeout 300 cmake -S . -B build && timeout 300 cmake --build build + - name: memcheck against the fixed invocation set + run: timeout 300 scripts/valgrind.sh "$(pwd)/build/doos" + - name: keep the per-step logs on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: valgrind-reports + path: build/valgrind-reports/ + + # Real overlayfs, real user/mount/pid namespaces, real cgroup v2 limits, real downloads: needs + # the self-hosted host-mode runner set up per docs/CI.md (label "doos-host"), not a container. + test: + runs-on: [self-hosted, doos-host] + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + - name: build + run: timeout 300 cmake -S . -B build && timeout 300 cmake --build build + - name: end-to-end suite + run: timeout 600 tests/run.sh "$(pwd)/build/doos" + + # Named CVE regression set against the binary this run just built. Same runner requirement as + # `test`, for the same reason (several checks attach a real container). + audit: + runs-on: [self-hosted, doos-host] + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + - name: build + run: timeout 300 cmake -S . -B build && timeout 300 cmake --build build + - name: CVE regression checks + run: timeout 300 make -C audit -j8 test DOOS="$(pwd)/build/doos" diff --git a/AGENTS.md b/AGENTS.md index 0829953..c027b14 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ # AGENTS.md -**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) +**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) · [CI](docs/CI.md) Rules for any AI agent (or human) changing this repository. The user's global rules also apply; this file adds the project-specific ones. Keep this file current: it is @@ -54,7 +54,8 @@ Never hand-edit `VERSION` or create tags manually. Never skip the tests to relea | Image catalog, sources, verification, pinned keys | `SPEC.md` Images, `README.md`, `docs/RESEARCH.md` | | Seccomp list, capability set, `/etc` allowlist | `SPEC.md` Hardening and filesystem, `README.md`, `docs/RESEARCH.md` | | Limits, cgroup layout | `SPEC.md` Resource limits, `docs/ARCHITECTURE.md` | -| Anything above, in every release | `README.md` version line and `CHANGELOG.md` entry | +| Anything above, in every release | `CHANGELOG.md` entry (`README.md` points at `VERSION`, nothing to edit there) | +| The CI workflow or what a job needs | `.gitea/workflows/ci.yml`, `docs/CI.md` | Docs describe what the system does today. No roadmap text. A gap is listed next to what would close it. A number in the docs needs a run behind it, else say it is unmeasured. @@ -84,6 +85,13 @@ freeze (about 4 minutes in total, needs network, `pasta`, `gpg` and cgroup v2 de before release. On Ubuntu 24.04+ the AppArmor profile in `apparmor/doos` must be installed once for the binary being tested (`doos apparmor`; needs sudo, ask the user, do not do it yourself). +## CI + +`.gitea/workflows/ci.yml` runs `build`, `valgrind`, `test` and `audit` on every push (Gitea Actions). +`test` and `audit` need the self-hosted host-mode runner described in [docs/CI.md](docs/CI.md) (real +namespaces, cgroup v2 delegation, the AppArmor profile); the workflow itself never runs `sudo`, same +rule as everywhere else. See that file before changing the workflow or what a job needs. + ## Make targets `make help` lists them: `build`, `test`, `audit` (named CVE regression checks against `build/doos`, diff --git a/CHANGELOG.md b/CHANGELOG.md index c088339..8f2bb63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ # Changelog -**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) +**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) · [CI](docs/CI.md) + +## 1.3.2 - 2026-10-24 +- CI: `.gitea/workflows/ci.yml` (Gitea Actions) runs `build`, `valgrind`, `test` and `audit` on every push. `build`/`valgrind` run on any Docker-mode runner; `test`/`audit` need a self-hosted host-mode runner (real namespaces, cgroup v2 delegation, the AppArmor profile), documented with its one-time setup in the new [docs/CI.md](docs/CI.md). Every step wraps its own `timeout`, since Gitea Actions parses but does not enforce `timeout-minutes`/`concurrency` (checked against the 1.25 docs). +- Fixed, from [review2.md](review2.md): `parseSize` (`src/util.cpp`) overflowed silently on an oversized input (`--size 16777217t` was accepted as exactly `1.0T` instead of rejected); now checked before the shift and rejected. Tracked as gap G9 in [RESEARCH](docs/RESEARCH.md); regression case added to `tests/run.sh`. +- Fixed, from [review2.md](review2.md): `newuidmap`, `newgidmap`, `slirp4netns` and `pasta` were each pinned to one hardcoded `/usr/bin/...` path; they now go through `resolveBinary()` (state-dir override, then a fixed list of system directories), the same resolution `curl`/`tar`/`sha256sum`/`gpg` already used. Tracked as gap G10. +- Docs: the `setns`-before-hardening ordering invariant in `Sandbox::join` and `Sandbox::start` (seccomp denies `setns`, and a filter is inherited for life) is now a comment at both call sites, not only in reviewer heads. The hand-rolled Alpine/Debian catalog parsers are logged as an accepted trade-off, gap G11, rather than left undocumented. +- Added `LICENSE` (MIT). +- Docs: README's two overlapping, partly stale intro blocks (a leftover `currently 1.1.3` version line, a narrower duplicate usage example) collapsed into one accurate one. ## 1.3.1 - 2026-10-24 - Docs: gap G8 (`fsutil::mkdirs` following a planted symlink, [RESEARCH](docs/RESEARCH.md)) marked fixed, re-measured with `make -C audit/CVE-2021-30465 test` (PASS); removed from [COMPARISON](COMPARISON.md)'s gaps list. The fix itself shipped in 1.3.0 as part of the security-review hardening; this corrects the docs to match. diff --git a/COMPARISON.md b/COMPARISON.md index 5dc57ac..e667f83 100644 --- a/COMPARISON.md +++ b/COMPARISON.md @@ -1,6 +1,6 @@ # doos compared with Docker, Podman and others -**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) +**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) · [CI](docs/CI.md) This page puts doos next to the tools people usually reach for instead. It separates three kinds of claims, and every row says which one it is: diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..8e1e6a4 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 retoor + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index aade79c..4647c5f 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # doos -**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) +**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) · [CI](docs/CI.md) User-level containers in C++20: overlayfs, user/mount/pid namespaces, slirp4netns or pasta networking, optional images (Alpine, Ubuntu, Debian, or your own rootfs), memory/cpu/pid/size limits, default hardening (capabilities, seccomp). No root at runtime. @@ -13,16 +13,7 @@ doos image list | add [--sha256 HEX] SOURCE [NAME] | delete [--yes] NAME doos apparmor # profile text for this binary (Ubuntu 24.04+) ``` -Version: see `VERSION` (currently 1.1.3). Release history: `CHANGELOG.md`. - -User-level containers in C++20: overlayfs over the host's `/usr`, `/etc`, `/opt`, user/mount/pid namespaces, networking through slirp4netns. No root at runtime. - -``` -doos list -doos get_or_create [--net slirp|host|none] NAME -doos attach [--net slirp|host|none] NAME [-- CMD...] -doos delete [--yes] NAME -``` +Version: see `VERSION`. Release history: [CHANGELOG.md](CHANGELOG.md). License: [MIT](LICENSE). Build: `cmake -S . -B build && cmake --build build`. Test: `tests/run.sh`. @@ -176,7 +167,7 @@ Details: [COMPARISON](COMPARISON.md) section 6.1. ## Development -`make help` lists the targets: `make build`, `make test` (end-to-end, needs network), `make audit` (named CVE regression checks, see [audit/README.md](audit/README.md)), `make valgrind` (memcheck against a fixed set of invocations, see [valgrind/README.md](valgrind/README.md)), `make bench` (re-measure startup, sizes and the cpu limit), `make charts` (rebuild the SVG charts and the generated numbers in the comparison), `make docs`, `make release KIND=patch MSG="..."`. Build and test details are in [AGENTS](AGENTS.md); the charts and how they are generated are in [COMPARISON](COMPARISON.md) section 3 and `docs/charts/`. +`make help` lists the targets: `make build`, `make test` (end-to-end, needs network), `make audit` (named CVE regression checks, see [audit/README.md](audit/README.md)), `make valgrind` (memcheck against a fixed set of invocations, see [valgrind/README.md](valgrind/README.md)), `make bench` (re-measure startup, sizes and the cpu limit), `make charts` (rebuild the SVG charts and the generated numbers in the comparison), `make docs`, `make release KIND=patch MSG="..."`. Build and test details are in [AGENTS](AGENTS.md); the charts and how they are generated are in [COMPARISON](COMPARISON.md) section 3 and `docs/charts/`. CI (`.gitea/workflows/ci.yml`) runs `build`, `valgrind`, `test` and `audit` on every push; see [docs/CI.md](docs/CI.md) for what each needs and the one-time runner setup. ## Project docs @@ -190,5 +181,6 @@ Details: [COMPARISON](COMPARISON.md) section 6.1. | [`CHANGELOG.md`](CHANGELOG.md) | Release history | | [`audit/README.md`](audit/README.md) | Named CVE regression checks run against `build/doos` (`make audit`) | | [`valgrind/README.md`](valgrind/README.md) | Memcheck against a fixed set of invocations (`make valgrind`) | +| [`docs/CI.md`](docs/CI.md) | Gitea Actions: what runs where, runner setup | Not sure whether doos is the right tool? Read the conclusion of [COMPARISON](COMPARISON.md) first. diff --git a/SPEC.md b/SPEC.md index 5dc6076..f3503b5 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1,6 +1,6 @@ # doos specification -**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) +**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · [RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · [CHANGELOG](CHANGELOG.md) · [CI](docs/CI.md) Authoritative description of behavior and limits. Code constants live in `src/spec.hpp`; change both together. How the code implements it: [ARCHITECTURE](docs/ARCHITECTURE.md). Measurements and remaining limits: [RESEARCH](docs/RESEARCH.md). How it differs from other tools: [COMPARISON](COMPARISON.md). Usage: [README](README.md). diff --git a/VERSION b/VERSION index 3a3cd8c..88c5fb8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.1 +1.4.0 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2af1f2e..d821583 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1,6 +1,6 @@ # Architecture -**Docs:** [README](../README.md) · [SPEC](../SPEC.md) · [ARCHITECTURE](ARCHITECTURE.md) · [RESEARCH](RESEARCH.md) · [COMPARISON](../COMPARISON.md) · [AGENTS](../AGENTS.md) · [CHANGELOG](../CHANGELOG.md) +**Docs:** [README](../README.md) · [SPEC](../SPEC.md) · [ARCHITECTURE](ARCHITECTURE.md) · [RESEARCH](RESEARCH.md) · [COMPARISON](../COMPARISON.md) · [AGENTS](../AGENTS.md) · [CHANGELOG](../CHANGELOG.md) · [CI](CI.md) How doos works internally. The behavior contract is [SPEC](../SPEC.md); this explains the code. Why it was built this way and what was measured: [RESEARCH](RESEARCH.md); how it compares with other tools: [COMPARISON](../COMPARISON.md). diff --git a/docs/CI.md b/docs/CI.md new file mode 100644 index 0000000..0547711 --- /dev/null +++ b/docs/CI.md @@ -0,0 +1,87 @@ +# CI: Gitea Actions + +**Docs:** [README](../README.md) · [SPEC](../SPEC.md) · [ARCHITECTURE](ARCHITECTURE.md) · [RESEARCH](RESEARCH.md) · [COMPARISON](../COMPARISON.md) · [AGENTS](../AGENTS.md) · [CHANGELOG](../CHANGELOG.md) · [CI](CI.md) + +The workflow is [`.gitea/workflows/ci.yml`](../.gitea/workflows/ci.yml). It needs two kinds of +runner, because the thing doos tests is the thing most CI containers are built to prevent: a +process creating its own user, mount and pid namespaces and an overlayfs mount. + +## What runs where + +| Job | Runner | Needs | Proves | +|---|---|---|---| +| `build` | any `docker://` runner (label `ubuntu-latest`) | a C++20 toolchain and CMake | the binary still compiles with `-Werror` and the documentation set is still fully cross-linked | +| `valgrind` | any `docker://` runner (label `ubuntu-latest`) | the above, plus `valgrind` | no new memory error or leak in doos's own code (see [valgrind/README.md](../valgrind/README.md)); this still gives a real answer even when the container's own AppArmor/seccomp policy blocks the user namespace doos tries to create, because `scripts/valgrind.sh` only fails on what valgrind itself reports, never on doos's own exit code | +| `test` | self-hosted, host mode, label `doos-host` | see below | the real end-to-end suite (`tests/run.sh`, currently 88 checks): real overlayfs, real namespaces, real downloads, real cgroup v2 limits | +| `audit` | self-hosted, host mode, label `doos-host` | see below | the named CVE regression set in [`audit/`](../audit/README.md) against the binary this run just built | + +`build` and `valgrind` run in an ephemeral Docker-mode act_runner container and need nothing this +repository does not already ask of any build machine. `test` and `audit` need a real Linux host +(or VM) that act_runner reaches in *host mode* (a plain label, not a `docker://` one), because the +things under test are precisely the things a container runtime's own default policy restricts: an +unprivileged user namespace that creates an overlayfs mount, a systemd user manager with cgroup v2 +delegated to it, and (on Ubuntu 24.04+) an AppArmor profile that permits user namespaces for this +one binary path. Running those two jobs inside another container would either need that container +privileged (defeating the point of isolating the runner) or would silently test less than it claims +to, the same vacuous-pass risk `review2.md` section 2 warns about for the audit script's own design. + +## One-time host runner setup (by whoever administers the runner, not by the workflow) + +The workflow never runs anything with `sudo`, on purpose (same rule as everywhere else in this +project: AppArmor and system changes are requested from a human, never done by an agent or a CI +job on its own). Before registering the `doos-host` runner: + +``` +sudo apt-get install -y build-essential cmake slirp4netns passt gnupg uidmap curl tar +# /etc/subuid and /etc/subgid must list the runner's own user; uidmap's postinst usually does this. +grep "^$(whoami):" /etc/subuid /etc/subgid + +# cgroup v2 must be delegated to this user's systemd manager (loginctl enable-linger helps it +# survive without an interactive session): +loginctl enable-linger "$(whoami)" +cat /sys/fs/cgroup/user.slice/user-$(id -u).slice/user@$(id -u).service/cgroup.controllers + +# Build doos once at the exact path the runner will use, then install its AppArmor profile +# (needed on Ubuntu 24.04+; see README's Troubleshooting table for the symptom this fixes): +cmake -S . -B build && cmake --build build +build/doos apparmor | sudo tee /etc/apparmor.d/doos-runner >/dev/null +sudo apparmor_parser -r /etc/apparmor.d/doos-runner +``` + +Then register the runner itself (see +[Gitea's act_runner docs](https://docs.gitea.com/usage/actions/act-runner) for the current command +and token source): + +``` +./act_runner register --instance https:// --token \ + --name doos-host --labels doos-host +./act_runner daemon +``` + +A `docker://`-labelled runner for `build` and `valgrind` needs no special setup; any act_runner in +its default Docker mode works (the Gitea docs' own suggested registration labels, such as +`ubuntu-latest:docker://gitea/runner-images:ubuntu-latest`, are enough). + +## Known Gitea Actions gaps versus GitHub Actions (checked against the 1.25 docs) + +- **`timeout-minutes` and `concurrency` are parsed but ignored.** The workflow still sets + `timeout-minutes` on every job for readability and for anyone who later runs the same file + under real GitHub Actions, but the actual, enforced cap in each job is a shell-level + `timeout ` wrapped around the one command that matters, consistent with this project's + own rule that every automated process gets a hard, enforced ceiling, not a cap that only works + if the runner happens to honor it. +- **`permissions` support is version-dependent** and GitHub-only scopes (`checks`, `id-token`, + `security-events`, `pages`, ...) are not supported at all. The workflow does not rely on it. +- **`uses: actions/checkout@v4`** is resolved the same way GitHub Actions resolves it (Gitea + Actions proxies to GitHub by default); an instance configured without internet access to + GitHub needs `https://gitea.com/actions/checkout` instead, or a plain `git clone` step. + +## Running it without Gitea + +Every job is one or two `make` targets with a `timeout` in front; none of it depends on Gitea +Actions syntax beyond `on`/`jobs`/`runs-on`/`steps`, so the same file works, unmodified, as a +GitHub Actions workflow (just move it to `.github/workflows/`) with the one exception that +`timeout-minutes` would then actually be enforced by the platform instead of being a label only +(harmless either way). + +Back to [README](../README.md), [AGENTS](../AGENTS.md), [SPEC](../SPEC.md). diff --git a/docs/RESEARCH.md b/docs/RESEARCH.md index 1c27efc..71807d2 100644 --- a/docs/RESEARCH.md +++ b/docs/RESEARCH.md @@ -1,6 +1,6 @@ # Research: user-level containers with overlayfs -**Docs:** [README](../README.md) · [SPEC](../SPEC.md) · [ARCHITECTURE](ARCHITECTURE.md) · [RESEARCH](RESEARCH.md) · [COMPARISON](../COMPARISON.md) · [AGENTS](../AGENTS.md) · [CHANGELOG](../CHANGELOG.md) +**Docs:** [README](../README.md) · [SPEC](../SPEC.md) · [ARCHITECTURE](ARCHITECTURE.md) · [RESEARCH](RESEARCH.md) · [COMPARISON](../COMPARISON.md) · [AGENTS](../AGENTS.md) · [CHANGELOG](../CHANGELOG.md) · [CI](CI.md) Measured on this machine: Ubuntu, kernel 7.0.0-38, AppArmor 5.0.2, slirp4netns 1.3.3, uid 1000, no root at runtime. @@ -63,6 +63,9 @@ removed afterwards. Rows marked "unmeasured" have no run behind them. | G6 | No seccomp, nested userns allowed | Capability drop + seccomp-bpf denylist | **done in 1.1.0** (`--hardening`) | `hardening.cpp` | | G7 | Host-owned lower dirs unwritable, `chown` of host files fails, empty `/var` | Image layer + subuid; skeleton and seeding for host mode | **done in 1.0.0** | `image.cpp`, `container.cpp` | | G8 | `fsutil::mkdirs` followed a symlink planted at a container's path under `$DOOS_HOME/containers/`, same bug class as [`audit/CVE-2021-30465`](../audit/CVE-2021-30465/README.md) | walk the path component by component from a pinned base fd with `mkdirat`/`openat(O_NOFOLLOW\|O_DIRECTORY)`, refusing any symlink instead of following it | **done in 1.3.0**, re-measured: `make -C audit/CVE-2021-30465 test` now PASSes | `fsutil::mkdirs` in `src/util.cpp` | +| G9 | `parseSize` left-shifted without an overflow check: `--size 16777217t` wrapped to exactly `1.0T` and was accepted instead of rejected | check `v > (UINT64_MAX >> shift)` before shifting, reject instead of wrapping | **done in 1.3.2**, reproduced before the fix, re-run after: rejected | `parseSize` in `src/util.cpp` | +| G10 | `newuidmap`/`newgidmap`/`slirp4netns`/`pasta` were pinned to a single hardcoded `/usr/bin/...` path, unlike `curl`/`tar`/`sha256sum`/`gpg` which go through `resolveBinary()`'s state-dir-override-then-fixed-dirs search; a distro installing them elsewhere would fail even though PATH-poisoning was never possible either way | route all four through `resolveBinary()`, same as the other helpers | **done in 1.3.2** | `idmap.cpp`, `network.cpp` | +| G11 | Alpine's `latest-releases.yaml` and Debian's `index.json` (embedded base64 manifest) are read with fixed string/byte-offset search, not a YAML/JSON library; a publisher reformatting either file breaks extraction with a thrown error (fails closed) rather than silently misreading it | none: a dependency to parse two small, publisher-controlled files is a worse trade than failing loudly when their format changes, given the project's own "fewer dependencies" rule | **accepted, not fixed**; this row exists so the trade-off is written down, not rediscovered | `Catalog::resolve` in `src/image.cpp` | ### G1: memory, cpu and pids limits (cgroup v2) diff --git a/review2.md b/review2.md new file mode 100644 index 0000000..cfb72d5 --- /dev/null +++ b/review2.md @@ -0,0 +1,150 @@ +# doos: full review + +Date: 2026-10-24 | Scope: all of `src/` (3,558 lines, read in full), `CMakeLists.txt`, `Makefile`, +`scripts/*.sh`, `tests/run.sh`, `audit/`, `valgrind/`, and every document (`README.md`, `SPEC.md`, +`docs/ARCHITECTURE.md`, `docs/RESEARCH.md`, `COMPARISON.md`, `AGENTS.md`, `CHANGELOG.md`). +Method: read every source file top to bottom (not a diff), ran the real e2e suite, the CVE audit, +and valgrind against the current binary, and tested two inputs by hand that looked suspicious while +reading. This is a second, independent pass; [`review.md`](review.md) is an earlier review whose +findings are checked against the current code, not reused. + +**State at review time:** `v1.3.1`, working tree clean, `tests/run.sh` 88/88, `make audit` 18/18, +`make valgrind` 6/6 (see those tools for what they do and do not cover). + +## 1. Summary + +doos is a small, coherent, unusually well-specified codebase for what it is: a single C++20 binary +that builds a user-namespace container by hand (no runc, no libcontainer, no OCI anything) and +backs it with overlayfs, cgroup v2 and a seccomp denylist. The code reads like it was written by one +disciplined author following one set of rules consistently: every constant lives in `spec.hpp`, every +destructive path is capped, every external process is run from a fixed path, every error throws with +a message that says what to do about it. The project-level discipline (docs that state gaps next to +the code that closes them, a changelog per release, a `make audit`/`make valgrind` pair that produces +actual evidence instead of claims) is better than most projects an order of magnitude larger. + +The flip side of "one author, one style" is that nothing here has been read by a second pair of eyes +until review.md and this review. I found one confirmed, reproducible bug (an integer overflow that +silently accepts a nonsensical `--size`) and a handful of smaller inconsistencies and gaps, listed +below. Nothing I found is a new security hole in the model doos is actually built on; the shared-kernel, +denylist-seccomp, unaudited-code risks are already disclosed in `COMPARISON.md` section 6 and +`docs/RESEARCH.md`, and I did not find reason to soften or harden that disclosure. + +**Resolution (1.3.2, same day):** 2.1, 2.2 and 2.4 are fixed (see `CHANGELOG.md`); 2.3 is logged as +an accepted trade-off (gap G11 in `docs/RESEARCH.md`) rather than fixed; 2.5's CI half is resolved +by `.gitea/workflows/ci.yml` and `docs/CI.md`, and its LICENSE half by the new `LICENSE` file. The +findings below are left exactly as first written, for the record of what was found and how. + +## 2. Findings + +Ordered most to least severe. Each is something I read or reproduced, not a hypothetical. + +### 2.1 [MEDIUM] `parseSize` silently overflows on an absurd input, instead of rejecting it + +`src/util.cpp:54-71`. The function accepts up to 15 digits, then left-shifts by up to 40 bits with no +overflow check. `--size 16777217t` (16.7 million exabytes, obvious garbage) wraps around 64-bit +arithmetic to exactly `2^40`, which is `1.0T`, which is a *valid* size, so it is accepted silently: + +``` +$ doos get_or_create --size 16777217t sizetest2 +created sizetest2 +$ doos list +NAME STATE NET IMAGE SIZE AGE +sizetest2 stopped slirp host 1.0T 0s +``` + +Reproduced against `build/doos` at the time of this review. The same shift pattern exists for `k`/`m`/`g` +too, just with smaller multipliers that need more digits to wrap; `t` wraps with an 8-digit input. Every +caller does bound-check the *result*, which is why most overflowing inputs are still rejected (the +earlier example, `16777217t`+1 lands outside range and throws): it is specifically the inputs that +wrap into the valid `[64m, 1t]` window that go through unnoticed. Fix: check `v > (kMaxSizeBytes >> +shift)` before shifting, or reject more than 4-5 digits before a `t`/`g` suffix. + +### 2.2 [LOW] Two different "no PATH lookup" strategies for external helpers, not one + +`spec.hpp:52-53,75-76` hardcode `/usr/bin/newuidmap`, `/usr/bin/newgidmap`, `/usr/bin/slirp4netns`, +`/usr/bin/pasta` as single absolute paths. `resolveBinary()` (`src/util.cpp:397-417`) instead checks a +state-dir override then a list of six system directories, and is used for `curl`, `tar`, `sha256sum` +and `gpg`. Both avoid a `PATH`-poisoning attack (the actual goal, and both reach it), but the first +group breaks if a distribution installs those four tools somewhere other than exactly `/usr/bin` (for +example Fedora's `/usr/sbin` for some setuid helpers, or a non-standard prefix), while the second group +would still find them. This is not a security gap, it is an inconsistency that will show up as a +confusing "helper not found" on a distro the project has not been tested on, where the real fix (run +`resolveBinary()` for all five, or document the four as intentionally pinned) is a few lines either way. + +### 2.3 [LOW] Catalog YAML/JSON parsing is hand-rolled string search, not a parser + +`src/image.cpp:160-173` (Alpine's `latest-releases.yaml`) and `src/image.cpp:192-204` (Debian's +`index.json`, including a base64-embedded manifest) are read with `line.find(...)` and fixed byte +offsets, not a YAML/JSON library. This keeps the dependency count at zero, which fits the project's +"fewer moving parts" rule, and it is already tested against the real Alpine and Debian endpoints in +`tests/run.sh`. The risk is narrow but real: if either publisher reformats their file (reorders keys, +changes indentation, wraps a line), the parser fails closed with a thrown error (good: `docs/RESEARCH.md` +already calls this an accepted trade-off for Debian specifically) rather than silently extracting the +wrong artifact. Worth a one-line gap entry next to the code, same as the project's own convention asks. + +### 2.4 [LOW] `Sandbox::join`'s ordering is a correctness invariant that is not stated in code + +`src/sandbox.cpp:521-581`. Joining a running container does six `setns()` calls in a plain (not yet +seccomp-filtered) forked process, then forks again and *only there* calls `applyLimitsAndEnv`, which +installs the seccomp filter that denies `setns` (`src/hardening.cpp:184-186`). This is correct and +I could not find a bug in it, but the correctness depends entirely on hardening being applied after +the `setns` loop and in the right (inner) fork, a constraint that lives only in the order of statements, +not in a comment or an assertion. A future edit that moves `applyLimitsAndEnv` earlier "to simplify" +would silently break every second `attach`, and the e2e suite's "second attach joins" case would need +to be the thing that catches it rather than a comment telling the next editor not to do that. + +### 2.5 [LOW] No CI, no license file + +`find . -iname "*.yml"` turns up nothing; there is no `.github/workflows`, so the 88-check e2e suite, +`make audit` and `make valgrind` only run when someone remembers to run them by hand. There is also no +`LICENSE` file, which for a "commercial and scientific" project (the user's own standing rule) means +the legal default is "all rights reserved" by anyone who is not the author, worth a deliberate choice +rather than an omission. + +### 2.6 [INFO] Everything else I checked was either already documented or held up + +I specifically went looking for bugs in the capability-drop/seccomp-install ordering relative to +`execve`, in the overlayfs mount option construction, in the `--size`/`--memory`/`--pids`/`--cpus` +range validation (other than 2.1), in the container/image name validators against path traversal, in +`Store::remove`'s delete-outside-the-store guard, and in the symlink handling added for the PATH and +TOCTOU fixes (`src/util.cpp:329-354`, `src/sandbox.cpp:130-153`). All of it matched what the comments +claim, and the specific "mkdirs follows a symlink" class of bug this project itself found via +`audit/CVE-2021-30465` is closed the right way (per-component `openat(O_NOFOLLOW)`), not patched over. + +## 3. Grades + +| Category | Grade | Why | +|---|---|---| +| Architecture & code organization | 8.5/10 | One class per responsibility, matches `docs/ARCHITECTURE.md` exactly; every constant in one file; consistent error style throughout 3,558 lines. Loses points only for 2.4's unstated invariant. | +| Correctness & input validation | 7/10 | Careful everywhere except one real bug (2.1). CLI parsing, name validation, bounds checks on memory/pids/cpus are all solid; size parsing is the one spot that was not. | +| Isolation design (namespaces, overlayfs, mounts) | 9/10 | TOCTOU-safe symlink handling, `O_NOFOLLOW` directory creation, correct remount-flag preservation, correct setns/seccomp ordering. This is the part of the codebase that most clearly got the most attention, and it shows. | +| Resource controls (cgroup, size cap) | 8.5/10 | Adaptive scan-duty throttling, freeze/thaw hysteresis, whole-container OOM kill, hard caps on scan size and tree depth. Honestly scoped: documented as advisory without cgroup v2 delegation rather than pretending to enforce. | +| Supply chain (image verification) | 8/10 | Real signature verification against pinned fingerprints for two of three catalog sources, sha256 for all three, URL inputs refused without a hash. Loses points for the hand-rolled parsers (2.3) and for Debian having no publisher signature (disclosed, not doos's fault, but still the weakest link). | +| Hardening (capabilities, seccomp) | 8/10 | Matches Docker's default capability mask (measured, not assumed), denylist seccomp with the known evasions (x32, clone3, nested userns) explicitly closed. A denylist is inherently weaker than an allowlist by design, which the project says itself in `COMPARISON.md`; grade reflects execution of the chosen design, not the design choice. | +| Testing & verification tooling | 8.5/10 | 88 end-to-end checks through the real path (real overlayfs, real downloads, real cgroups), plus `audit/` (18 named CVEs with working reproducers) and `valgrind/` (6 checks, suppressions individually justified) are well above what a project this size usually has. One machine, one architecture, no CI (2.5) is the honest ceiling. | +| Documentation | 9/10 | Seven cross-linked documents that state what exists today, with gaps listed next to the code that closes them; generated numbers and charts instead of hand-typed ones; a changelog that actually matches releases. Among the best-documented small C++ projects I have reviewed. | +| Build & release automation | 8.5/10 | `scripts/release.sh` bumps, builds, tests, checks docs, commits and tags in one gated command and aborts on any failure; hardened compiler/linker flags. Loses half a point for no CI to run any of it automatically. | +| CLI/UX | 7.5/10 | Clear, narrow-terminal-friendly usage text; sensible fixed-vs-changeable option model; good error messages that say what to do next. `--i-understand` as a deliberate confirmation gate for reduced sandboxing is a good pattern. | +| Project hygiene | 6/10 | No LICENSE, no CI (2.5); otherwise clean (`.gitignore` is thorough, no stray build artifacts tracked, commit messages are consistent and attributed). | + +**Overall: 8/10.** A small, honest, carefully built tool with one real bug found by testing it rather +than reading it, and a short, specific list of things worth fixing. Nothing found here changes the +conclusion already in `COMPARISON.md`: fine for your own tools, not a substitute for an audited, +multi-tenant-hardened runtime. + +## 4. Recommended order of fixes + +1. Fix the `parseSize` overflow (2.1): cheapest fix, only confirmed bug, trivial to test. +2. Add a one-line comment above `applyLimitsAndEnv`'s two call sites in `sandbox.cpp` stating the + ordering invariant from 2.4, so the next edit does not have to rediscover it the hard way. +3. Decide and document the helper-resolution split (2.2): either move the four pinned paths through + `resolveBinary()`, or add one line to `SPEC.md` saying why they are intentionally narrower. +4. Add a LICENSE file (2.5); pick whichever the user already uses elsewhere. +5. CI is the only item here that is genuinely more than a few lines of work (namespaces, cgroup v2 + delegation and AppArmor all need to exist on the runner); worth a separate, explicit decision + rather than a drive-by addition. + +**Docs:** [README](README.md) · [SPEC](SPEC.md) · [ARCHITECTURE](docs/ARCHITECTURE.md) · +[RESEARCH](docs/RESEARCH.md) · [COMPARISON](COMPARISON.md) · [AGENTS](AGENTS.md) · +[CHANGELOG](CHANGELOG.md) · [audit](audit/README.md) · [valgrind](valgrind/README.md) · +[review.md](review.md) (earlier security-only review) diff --git a/scripts/check-docs.sh b/scripts/check-docs.sh index 6bd2145..60036a5 100755 --- a/scripts/check-docs.sh +++ b/scripts/check-docs.sh @@ -4,7 +4,7 @@ set -u cd "$(dirname "$0")/.." -DOCS=(README.md SPEC.md COMPARISON.md AGENTS.md CHANGELOG.md docs/ARCHITECTURE.md docs/RESEARCH.md) +DOCS=(README.md SPEC.md COMPARISON.md AGENTS.md CHANGELOG.md docs/ARCHITECTURE.md docs/RESEARCH.md docs/CI.md) bad=0 fail() { echo "docs: $*"; bad=$((bad+1)); } @@ -12,11 +12,11 @@ fail() { echo "docs: $*"; bad=$((bad+1)); } while IFS= read -r f; do f=${f#./} printf '%s\n' "${DOCS[@]}" | grep -qxF "$f" || fail "$f is not in the DOCS list of scripts/check-docs.sh" -done < <(find . -name '*.md' -not -path './build/*' -not -path './.git/*' -not -path './prompt.md' -not -path './audit/*' -not -path './review.md' -not -path './valgrind/*') +done < <(find . -name '*.md' -not -path './build/*' -not -path './.git/*' -not -path './prompt.md' -not -path './audit/*' -not -path './review.md' -not -path './review2.md' -not -path './valgrind/*') # prompt.md: the original task statement, not documentation. # audit/*, valgrind/*: separate, self-contained doc sets (one README per CVE check, one for the # valgrind setup); linked from the main docs but not forced into their full cross-link mesh. -# review.md: a standalone external security review, not part of the maintained doc set. +# review.md, review2.md: standalone one-off reviews, not part of the maintained doc set. for f in "${DOCS[@]}"; do [ -f "$f" ] || { fail "$f is missing"; continue; } diff --git a/src/idmap.cpp b/src/idmap.cpp index c1ea749..3f1413e 100644 --- a/src/idmap.cpp +++ b/src/idmap.cpp @@ -20,6 +20,16 @@ namespace doos { namespace { +// Same trusted-path resolution as every other external helper (resolveBinary in util.cpp): the +// state dir's bin/ override, then a fixed list of system directories, never the caller's PATH. +std::string findBinary(const char* name) { + try { + return resolveBinary(name); + } catch (const std::exception&) { + return {}; + } +} + void writeProcFile(pid_t pid, const char* file, const std::string& content) { std::string path = "/proc/" + std::to_string(pid) + "/" + file; int fd = ::open(path.c_str(), O_WRONLY | O_CLOEXEC); @@ -61,8 +71,8 @@ std::optional IdMap::range(const char* file) { } bool IdMap::subuidAvailable() { - return range("/etc/subuid") && range("/etc/subgid") && ::access(std::string(spec::kNewUidMap).c_str(), X_OK) == 0 && - ::access(std::string(spec::kNewGidMap).c_str(), X_OK) == 0; + return range("/etc/subuid") && range("/etc/subgid") && !findBinary("newuidmap").empty() && + !findBinary("newgidmap").empty(); } IdMode IdMap::best() { return subuidAvailable() ? IdMode::Subuid : IdMode::Single; } @@ -77,15 +87,16 @@ void IdMap::apply(pid_t pid, IdMode mode) { } auto u = range("/etc/subuid"); auto g = range("/etc/subgid"); - if (!u || !g || ::access(std::string(spec::kNewUidMap).c_str(), X_OK) != 0 || ::access(std::string(spec::kNewGidMap).c_str(), X_OK) != 0) { + std::string newuidmap = findBinary("newuidmap"), newgidmap = findBinary("newgidmap"); + if (!u || !g || newuidmap.empty() || newgidmap.empty()) { throw std::runtime_error("subuid mapping unavailable: need /etc/subuid, /etc/subgid, newuidmap and newgidmap " "(install uidmap, or use --ids single)"); } auto n = [](const SubRange& r) { return std::to_string(std::min(r.count, spec::kMaxIdRange - 1)); }; std::string p = std::to_string(pid); - ProcResult a = runProcess({std::string(spec::kNewUidMap), p, "0", uid, "1", "1", std::to_string(u->start), n(*u)}); + ProcResult a = runProcess({newuidmap, p, "0", uid, "1", "1", std::to_string(u->start), n(*u)}); if (a.code != 0) throw std::runtime_error("newuidmap failed (exit " + std::to_string(a.code) + ")"); - ProcResult b = runProcess({std::string(spec::kNewGidMap), p, "0", gid, "1", "1", std::to_string(g->start), n(*g)}); + ProcResult b = runProcess({newgidmap, p, "0", gid, "1", "1", std::to_string(g->start), n(*g)}); if (b.code != 0) throw std::runtime_error("newgidmap failed (exit " + std::to_string(b.code) + ")"); } diff --git a/src/network.cpp b/src/network.cpp index 6c8ba2a..346fd76 100644 --- a/src/network.cpp +++ b/src/network.cpp @@ -50,9 +50,12 @@ std::string Network::resolvConf(NetMode m) { } void SlirpSession::start(pid_t targetPid) { - const std::string bin(spec::kSlirpBinary); - if (::access(bin.c_str(), X_OK) != 0) throw std::runtime_error("slirp4netns helper not found at " + bin + - " (install slirp4netns or use --net none)"); + std::string bin; + try { + bin = resolveBinary("slirp4netns"); + } catch (const std::exception&) { + throw std::runtime_error("slirp4netns helper not found (install slirp4netns or use --net none)"); + } int ready[2]; if (::pipe2(ready, O_CLOEXEC) != 0) throw SysError("pipe"); Fd readEnd(ready[0]), writeEnd(ready[1]); @@ -97,9 +100,12 @@ void SlirpSession::stop() { namespace doos { void PastaSession::start(pid_t targetPid) { - const std::string bin(spec::kPastaBinary); - if (::access(bin.c_str(), X_OK) != 0) throw std::runtime_error("pasta helper not found at " + bin + - " (install pasta or use --net none)"); + std::string bin; + try { + bin = resolveBinary("pasta"); + } catch (const std::exception&) { + throw std::runtime_error("pasta helper not found (install pasta or use --net none)"); + } pid_ = ::fork(); if (pid_ < 0) throw SysError("fork pasta"); if (pid_ == 0) { diff --git a/src/sandbox.cpp b/src/sandbox.cpp index cda1fb8..83bda8a 100644 --- a/src/sandbox.cpp +++ b/src/sandbox.cpp @@ -279,6 +279,10 @@ int containerInit(void* arg) { setupRoot(c, ctx->imageRoot); if (::sethostname(c.name().c_str(), c.name().size()) != 0) throw SysError("sethostname"); if (Network::isolated(c.net())) bringLoopbackUp(); + // Hardening (inside applyLimitsAndEnv) must be the last thing before the worker is forked + // and exec'd: everything this process still needs to do for itself (mounts, hostname, + // loopback) has to happen first, since its seccomp filter and dropped capabilities are + // inherited by every descendant from here on. See the matching note in Sandbox::join. applyLimitsAndEnv(c); pid_t worker = ::fork(); @@ -542,6 +546,11 @@ int Sandbox::join(pid_t initPid, FileLock& lock) { if (cgroup) { try { cgroup->addShell(::getpid()); } catch (const std::exception&) { /* limits only; keep going */ } } + // Ordering invariant: this setns loop must run before Hardening::apply() (inside + // applyLimitsAndEnv below), never after. The seccomp filter it installs denies + // SYS_setns outright (hardening.cpp's deniedSyscalls), and a seccomp filter is + // inherited by every descendant for the rest of its life. Moving the hardening call + // earlier would make every second `attach` fail to join a running container. for (int i = 0; i < 6; ++i) { if (fds[i].valid() && ::setns(fds[i].get(), kinds[i]) != 0) { throw SysError(std::string("setns ") + names[i]); @@ -549,7 +558,8 @@ int Sandbox::join(pid_t initPid, FileLock& lock) { } if (::fchdir(rootFd.get()) != 0 || ::chroot(".") != 0) throw SysError("chroot into container"); if (::chdir(std::string(spec::kHome).c_str()) != 0) throw SysError("chdir home"); - // setns(pid) only affects children, so fork once more. + // setns(pid) only affects children, so fork once more. Hardening (which the setns + // loop above must predate) is applied only in this inner child, right before exec. pid_t inner = ::fork(); if (inner < 0) throw SysError("fork"); if (inner == 0) { diff --git a/src/spec.hpp b/src/spec.hpp index f40659f..55b2a9c 100644 --- a/src/spec.hpp +++ b/src/spec.hpp @@ -47,10 +47,9 @@ constexpr std::size_t kMaxEtcCopyFiles = 50000; constexpr std::uint64_t kMaxEtcCopyBytes = 256ULL << 20; // Id mapping: container ids 0..kMaxIdRange. 0 maps to the invoking user, the rest to the subuid range. -// Helpers are invoked by absolute path (no PATH lookup) to avoid RCE via a poisoned PATH. +// newuidmap/newgidmap, like every other external helper, are found via resolveBinary() (util.cpp): +// the state dir's bin/ override, then a fixed list of system directories, never the caller's PATH. constexpr std::uint32_t kMaxIdRange = 65536; -constexpr std::string_view kNewUidMap = "/usr/bin/newuidmap"; -constexpr std::string_view kNewGidMap = "/usr/bin/newgidmap"; // Images constexpr std::size_t kMaxImages = 16; @@ -71,9 +70,8 @@ constexpr std::string_view kUbuntuKeyUrl = "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x843938DF228D22F7B3742BC0D94AA3F0EFE21092"; constexpr std::string_view kUbuntuKeyFpr = "843938DF228D22F7B3742BC0D94AA3F0EFE21092"; -// Networking -constexpr std::string_view kSlirpBinary = "/usr/bin/slirp4netns"; -constexpr std::string_view kPastaBinary = "/usr/bin/pasta"; +// Networking. slirp4netns and pasta are found via resolveBinary() (util.cpp), same as every other +// external helper: the state dir's bin/ override, then a fixed list of system directories. constexpr std::string_view kSlirpDns = "10.0.2.3"; constexpr std::string_view kSlirpTapName = "tap0"; diff --git a/src/util.cpp b/src/util.cpp index 23cf229..19c729c 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -61,11 +62,19 @@ std::uint64_t parseSize(const std::string& text) { if (unit == "b" || unit.empty()) { } else { if (unit.size() == 2 && unit[1] == 'b') unit.pop_back(); - if (unit == "k") v <<= 10; - else if (unit == "m") v <<= 20; - else if (unit == "g") v <<= 30; - else if (unit == "t") v <<= 40; + int shift; + if (unit == "k") shift = 10; + else if (unit == "m") shift = 20; + else if (unit == "g") shift = 30; + else if (unit == "t") shift = 40; else throw std::runtime_error("invalid size unit in '" + text + "' (use k, m, g or t)"); + // A 64-bit shift wraps silently on a large enough value instead of producing a huge + // number; reject anything that would not survive the shift intact rather than let it + // land, by chance, inside whatever range the caller validates against next. + if (v > (std::numeric_limits::max() >> shift)) { + throw std::runtime_error("size '" + text + "' is too large"); + } + v <<= shift; } return v; } diff --git a/tests/run.sh b/tests/run.sh index aaf943e..7942662 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -71,6 +71,7 @@ outs() { "$DOOS" attach s -- sh -c "$1" 2>&1; } check "ids fixed at creation" bash -c "! '$DOOS' get_or_create --ids subuid s1" check "bad size rejected" bash -c "! '$DOOS' get_or_create --size 1x z1" check "size below minimum rejected" bash -c "! '$DOOS' get_or_create --size 1m z2" +check "size overflow rejected, not silently wrapped" bash -c "! '$DOOS' get_or_create --size 16777217t z3" "$DOOS" attach s -- sh -c 'apt-get update >/dev/null 2>&1; apt-get install -y --no-install-recommends cowsay >/dev/null 2>&1; dpkg --audit && /usr/games/cowsay ok >/dev/null' && ok "host mode: apt install completes" || bad "host mode: apt install completes" [ ! -e /usr/games/cowsay ] && ok "host untouched by apt install" || bad "host untouched by apt install" "$DOOS" delete --yes s >/dev/null; "$DOOS" delete --yes s1 >/dev/null