commit 85f4d72210d79fa1a6841c7d0fab48343a28620e
parent 8e44a1d43cfe18ffe958c88434c62ee2fa6cef34
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Thu, 16 Jul 2026 10:12:44 -0700
cpio: validate archives before extraction
Diffstat:
| M | driver/cmd/cpio.c | | | 139 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------ |
| M | test/cpio/run.sh | | | 80 | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- |
2 files changed, 186 insertions(+), 33 deletions(-)
diff --git a/driver/cmd/cpio.c b/driver/cmd/cpio.c
@@ -201,9 +201,11 @@ static int cpio_parse(const uint8_t* data, size_t len, CpioVisit fn,
while (off + CPIO_HDR_LEN <= len) {
const uint8_t* h = data + off;
- uint32_t mode, filesize, namesize;
+ uint32_t fields[13];
+ uint32_t mode, filesize, namesize, check;
size_t name_off, data_off;
const char* name;
+ size_t i;
if (!cpio_is_magic(h)) {
driver_errf(CPIO_TOOL,
@@ -211,17 +213,21 @@ static int cpio_parse(const uint8_t* data, size_t len, CpioVisit fn,
: "corrupt cpio header (bad magic mid-stream)");
return 1;
}
- if (cpio_get_hex8(h + 14, &mode) || cpio_get_hex8(h + 54, &filesize) ||
- cpio_get_hex8(h + 94, &namesize)) {
- driver_errf(CPIO_TOOL, "corrupt cpio header (non-hex field)");
- return 1;
- }
+ for (i = 0; i < 13; ++i)
+ if (cpio_get_hex8(h + 6u + i * 8u, &fields[i])) {
+ driver_errf(CPIO_TOOL, "corrupt cpio header (non-hex field)");
+ return 1;
+ }
+ mode = fields[1];
+ filesize = fields[6];
+ namesize = fields[11];
+ check = fields[12];
if (namesize == 0) {
driver_errf(CPIO_TOOL, "corrupt cpio header (zero name size)");
return 1;
}
name_off = off + CPIO_HDR_LEN;
- if (name_off + namesize > len) {
+ if ((size_t)namesize > len - name_off) {
driver_errf(CPIO_TOOL, "truncated cpio archive (name runs past end)");
return 1;
}
@@ -230,9 +236,21 @@ static int cpio_parse(const uint8_t* data, size_t len, CpioVisit fn,
driver_errf(CPIO_TOOL, "corrupt cpio header (name not NUL-terminated)");
return 1;
}
+ if (name_off + (size_t)namesize > SIZE_MAX - 3u) {
+ driver_errf(CPIO_TOOL, "corrupt cpio header (name size overflow)");
+ return 1;
+ }
data_off = cpio_round4(name_off + namesize);
+ if (data_off > len) {
+ driver_errf(CPIO_TOOL, "truncated cpio archive (name padding missing)");
+ return 1;
+ }
if (namesize == 11u && memcmp(name, "TRAILER!!!", 11) == 0) {
+ if (filesize != 0) {
+ driver_errf(CPIO_TOOL, "corrupt cpio TRAILER!!! record");
+ return 1;
+ }
saw_trailer = 1;
off = data_off; /* filesize is 0 for the trailer */
/* Skip inter-segment zero padding; a following non-zero run that is a
@@ -242,18 +260,26 @@ static int cpio_parse(const uint8_t* data, size_t len, CpioVisit fn,
if (off >= len) break;
if (off + 6u > len || !cpio_is_magic(data + off)) {
driver_errf(CPIO_TOOL,
- "note: ignoring %lu trailing byte(s) after TRAILER (not a "
- "concatenated cpio archive)",
+ "corrupt cpio archive: %lu trailing byte(s) after TRAILER",
(unsigned long)(len - off));
- break;
+ return 1;
}
+ saw_trailer = 0; /* the concatenated segment needs its own trailer */
continue;
}
- if (data_off + filesize > len) {
+ if ((size_t)filesize > len - data_off) {
driver_errf(CPIO_TOOL, "truncated cpio archive (data runs past end)");
return 1;
}
+ if (memcmp(h, CPIO_MAGIC_CRC, 6) == 0) {
+ uint32_t actual = 0;
+ for (i = 0; i < filesize; ++i) actual += data[data_off + i];
+ if (actual != check) {
+ driver_errf(CPIO_TOOL, "corrupt cpio checksum for: %s", name);
+ return 1;
+ }
+ }
{
CpioEntry e;
e.name = name;
@@ -262,13 +288,21 @@ static int cpio_parse(const uint8_t* data, size_t len, CpioVisit fn,
e.filetype = cpio_ft_from_mode(mode);
e.data = data + data_off;
e.size = filesize;
- if (fn(user, &e)) return 1;
+ if (fn && fn(user, &e)) return 1;
+ }
+ if (data_off + (size_t)filesize > SIZE_MAX - 3u) {
+ driver_errf(CPIO_TOOL, "corrupt cpio header (data size overflow)");
+ return 1;
}
off = cpio_round4(data_off + filesize);
}
if (!saw_trailer) {
- driver_errf(CPIO_TOOL, "warning: archive has no TRAILER!!! record");
+ if (off < len)
+ driver_errf(CPIO_TOOL, "truncated cpio archive (incomplete header)");
+ else
+ driver_errf(CPIO_TOOL, "corrupt cpio archive: missing TRAILER!!! record");
+ return 1;
}
return 0;
}
@@ -573,8 +607,8 @@ void driver_help_cpio(void) {
"OPERANDS AND EXTRACTION\n"
" Create mode takes PATH operands; list/extract take no member\n"
" operands. Extraction writes below the current working directory,\n"
- " so change to the intended destination first. Prefix a create\n"
- " operand beginning with `-` by `./`; `--` is not accepted.\n"
+ " so change to the intended destination first. Use -- before a\n"
+ " create operand beginning with `-`.\n"
"\n"
"EXAMPLES\n"
" kit cpio -o -F root.cpio root/\n"
@@ -587,12 +621,6 @@ void driver_help_cpio(void) {
" kit cpio -o --lz4 root/ > root.cpio.lz4\n"
" kit cpio -i < root.cpio.lz4 # extract into cwd\n"
"\n"
- "KNOWN LIMITATION\n"
- " Malformed input and some conflicting mode combinations can\n"
- " currently diagnose an error but return status 0. Treat any cpio\n"
- " diagnostic as failure; the exit convention below is the intended\n"
- " contract after remediation.\n"
- "\n"
"EXIT CODES\n"
" 0 success 1 I/O or format error 2 bad usage\n")));
}
@@ -716,15 +744,44 @@ static int cpio_extract_visit(void* user, const CpioEntry* e) {
return 1;
}
} else {
- driver_errf(CPIO_TOOL, "skipping unsupported member type: %.*s",
+ driver_errf(CPIO_TOOL, "unsupported member type: %.*s",
(int)e->name_len, name);
- return 0;
+ return 1;
}
if (c->verbose) driver_printf("%.*s\n", (int)e->name_len, name);
return 0;
}
+/* Semantic validation runs over the complete archive before extraction starts,
+ * so an invalid later member cannot leave a partially trusted tree behind. */
+static int cpio_extract_validate(void* user, const CpioEntry* e) {
+ size_t i;
+ (void)user;
+ if (!cpio_name_safe(e->name, e->name_len)) {
+ driver_errf(CPIO_TOOL, "refusing unsafe member name: %.*s",
+ (int)e->name_len, e->name);
+ return 1;
+ }
+ if (e->filetype != 3 && e->filetype != 4 && e->filetype != 7) {
+ driver_errf(CPIO_TOOL, "unsupported member type: %.*s",
+ (int)e->name_len, e->name);
+ return 1;
+ }
+ if (e->filetype == 7) {
+ if (e->size >= CPIO_LINK_MAX) {
+ driver_errf(CPIO_TOOL, "symlink target too long: %s", e->name);
+ return 1;
+ }
+ for (i = 0; i < e->size; ++i)
+ if (e->data[i] == 0) {
+ driver_errf(CPIO_TOOL, "symlink target contains NUL: %s", e->name);
+ return 1;
+ }
+ }
+ return 0;
+}
+
/* ---------------------------------------------------------------------------
* Create
* ------------------------------------------------------------------------- */
@@ -951,9 +1008,16 @@ static int cpio_read(DriverEnv* env, KitContext* ctx, const CpioOpts* opt) {
payload_len = dlen;
} else if (opt->decompress) {
driver_errf(CPIO_TOOL, "-d given but input is not gzip/lz4-compressed");
- /* not fatal: fall through and parse as a raw archive */
+ goto done;
}
+ /* Structural validation is intentionally separate from presentation and
+ * extraction. For extraction the first pass also validates every path and
+ * supported member type before the first filesystem mutation. */
+ if (cpio_parse(payload, payload_len,
+ opt->mode == 'i' ? cpio_extract_validate : NULL, NULL) != 0)
+ goto done;
+
if (opt->mode == 't') {
CpioListCtx lc;
lc.verbose = opt->verbose;
@@ -994,6 +1058,16 @@ static int cpio_opt_arg(int argc, char** argv, int* i, const char* tool,
return 0;
}
+static int cpio_set_mode(CpioOpts* opt, char mode, const char* spelling) {
+ if (opt->mode && opt->mode != mode) {
+ driver_errf(CPIO_TOOL, "conflicting archive modes: -%c and %s", opt->mode,
+ spelling);
+ return 1;
+ }
+ opt->mode = mode;
+ return 0;
+}
+
int driver_cpio(int argc, char** argv) {
DriverEnv env;
KitContext ctx;
@@ -1028,11 +1102,11 @@ int driver_cpio(int argc, char** argv) {
if (driver_strneq(a, "--", 2)) { /* long option */
if (driver_streq(a, "--create")) {
- o.mode = 'o';
+ if (cpio_set_mode(&o, 'o', a)) goto done;
} else if (driver_streq(a, "--list")) {
- o.mode = 't';
+ if (cpio_set_mode(&o, 't', a)) goto done;
} else if (driver_streq(a, "--extract")) {
- o.mode = 'i';
+ if (cpio_set_mode(&o, 'i', a)) goto done;
} else if (driver_streq(a, "--verbose")) {
o.verbose = 1;
} else if (driver_streq(a, "--decompress")) {
@@ -1075,11 +1149,11 @@ int driver_cpio(int argc, char** argv) {
for (j = 1; a[j]; ++j) {
char ch = a[j];
if (ch == 'o') {
- o.mode = 'o';
+ if (cpio_set_mode(&o, 'o', "-o")) goto done;
} else if (ch == 't') {
- o.mode = 't';
+ if (cpio_set_mode(&o, 't', "-t")) goto done;
} else if (ch == 'i') {
- o.mode = 'i';
+ if (cpio_set_mode(&o, 'i', "-i")) goto done;
} else if (ch == 'v') {
o.verbose = 1;
} else if (ch == 'd') {
@@ -1130,8 +1204,9 @@ int driver_cpio(int argc, char** argv) {
}
if (o.mode != 'o' && nops > 0) {
driver_errf(CPIO_TOOL,
- "warning: member patterns are not supported; operating on the "
- "whole archive");
+ "member patterns are not supported: %s", ops[0]);
+ rc = 1;
+ goto done;
}
if (o.mode == 'o') {
diff --git a/test/cpio/run.sh b/test/cpio/run.sh
@@ -28,6 +28,28 @@ KIT_KIT_DIR="$repo_root/test/lib"
. "$repo_root/test/lib/kit_sh_kit.sh"
kit_report_init
+# Assert the documented 0/1/2 command classification, preserving stdout and
+# stderr beside the other lane artifacts when a case fails.
+expect_status() {
+ es_name=$1
+ es_want=$2
+ shift 2
+ "$@" > "$work/$es_name.out" 2> "$work/$es_name.err"
+ es_got=$?
+ if [ "$es_got" -eq "$es_want" ]; then
+ ok "$es_name"
+ else
+ {
+ printf 'status=%s (expected %s)\n' "$es_got" "$es_want"
+ sed 's/^/stdout: /' "$work/$es_name.out"
+ sed 's/^/stderr: /' "$work/$es_name.err"
+ } > "$work/$es_name.diag"
+ not_ok "$es_name" "$work/$es_name.diag"
+ fi
+}
+
+cpio_hdr() { printf '%s' "$1"; shift; for f in "$@"; do printf '%08X' "$f"; done; }
+
# ---- fixture: a small tree exercising every supported member type -----------
# regular file, executable regular file, zero-byte file, binary file, a nested
# subdirectory, an empty directory, and a symlink.
@@ -122,6 +144,35 @@ run_fail cpio-d-with-create create -o -d -F "$work/d.cpio" root
run_fail cpio-no-mode "$KIT" cpio -F "$work/out.cpio"
run_fail cpio-create-dotdot "$KIT" cpio -o -F "$work/dd.cpio" ../payload
+# Mode conflicts are usage errors; archive/decompression/pattern failures are
+# operational errors. Assert their exact classes instead of merely nonzero.
+expect_status cpio-mode-create-extract 2 "$KIT" cpio -o -i -F "$work/out.cpio"
+expect_status cpio-mode-create-list 2 "$KIT" cpio -o -t -F "$work/out.cpio"
+expect_status cpio-mode-extract-list 2 "$KIT" cpio -i -t -F "$work/out.cpio"
+expect_status cpio-explicit-decompress-raw 1 "$KIT" cpio -t -d -F "$work/out.cpio"
+expect_status cpio-unsupported-read-pattern 1 "$KIT" cpio -t -F "$work/out.cpio" root
+
+printf '070701' > "$work/truncated.cpio"
+expect_status cpio-truncated-header 1 "$KIT" cpio -t -F "$work/truncated.cpio"
+
+{
+ printf '999999'
+ dd if="$work/out.cpio" bs=1 skip=6 2>/dev/null
+} > "$work/bad-magic.cpio"
+expect_status cpio-bad-magic 1 "$KIT" cpio -t -F "$work/bad-magic.cpio"
+
+{
+ # One valid record with no TRAILER!!! record.
+ cpio_hdr 070701 0 33188 0 0 1 0 2 0 0 0 0 10 0
+ printf 'safe-file\000'
+ printf 'ok\000\000'
+} > "$work/no-trailer.cpio"
+expect_status cpio-missing-trailer 1 "$KIT" cpio -t -F "$work/no-trailer.cpio"
+
+cp "$work/out.cpio" "$work/trailing-garbage.cpio"
+printf 'not-an-archive' >> "$work/trailing-garbage.cpio"
+expect_status cpio-trailing-garbage 1 "$KIT" cpio -t -F "$work/trailing-garbage.cpio"
+
# ---- extract-time path-traversal safety ------------------------------------
# Hand-craft a newc archive whose sole member is named "../escape" and confirm
# extract refuses it (no file is written outside the destination). The 110-byte
@@ -129,7 +180,6 @@ run_fail cpio-create-dotdot "$KIT" cpio -o -F "$work/dd.cpio" ../payload
# mtime filesize devmajor devminor rdevmajor rdevminor namesize check) so the
# field widths can't drift.
ev="$work/evil.cpio"
-cpio_hdr() { printf '%s' "$1"; shift; for f in "$@"; do printf '%08X' "$f"; done; }
{
# mode 0100644 = 33188; name "../escape\0" = 10 bytes; data "pwn" = 3 bytes.
cpio_hdr 070701 0 33188 0 0 1 0 3 0 0 0 0 10 0
@@ -150,6 +200,34 @@ fi
"$KIT" cpio -t -F "$ev" > "$work/evil.list" 2>/dev/null
contains cpio-list-shows-dotdot "$work/evil.list" "../escape"
+# Validation is a complete first pass: a later unsafe member must prevent an
+# earlier, otherwise-valid member from being materialized.
+mixed="$work/mixed-evil.cpio"
+{
+ cpio_hdr 070701 0 33188 0 0 1 0 2 0 0 0 0 10 0
+ printf 'safe-file\000'
+ printf 'ok\000\000'
+ cpio_hdr 070701 0 33188 0 0 1 0 3 0 0 0 0 10 0
+ printf '../escape\000'
+ printf 'pwn\000'
+ cpio_hdr 070701 0 0 0 0 1 0 0 0 0 0 0 11 0
+ printf 'TRAILER!!!\000\000\000\000'
+} > "$mixed"
+mkdir -p "$work/ex-mixed"
+( cd "$work/ex-mixed" && "$KIT" cpio -i -F "$mixed" ) \
+ > "$work/mixed.out" 2> "$work/mixed.err"
+mixed_rc=$?
+if [ "$mixed_rc" -eq 1 ] && [ ! -e "$work/ex-mixed/safe-file" ]; then
+ ok cpio-prevalidate-before-extract
+else
+ {
+ printf 'status=%s safe-file=%s\n' "$mixed_rc" \
+ "$([ -e "$work/ex-mixed/safe-file" ] && printf present || printf absent)"
+ sed 's/^/stderr: /' "$work/mixed.err"
+ } > "$work/mixed.diag"
+ not_ok cpio-prevalidate-before-extract "$work/mixed.diag"
+fi
+
# ---- optional cross-check against the host cpio ----------------------------
if [ "${KIT_CPIO_TEST_HOST:-0}" = 1 ] && command -v cpio >/dev/null 2>&1; then
if cpio -t < "$work/out.cpio" > "$work/host.list" 2>/dev/null; then
