Hi,
I found two places in GNU tar 1.35 where bytes from a malicious archive
reach the terminal without going through quotearg, letting an attacker
inject ANSI escape sequences during normal use. Both reproduce on an
upstream-built tar 1.35 with default options.
Terminal escape injection via uname/gname in "tar -tvf"
The uname and gname fields from tar headers are printed raw through %s
at src/list.c:1278, bypassing the quotearg escaping applied to the
filename a few lines later. An attacker who puts ANSI escape bytes in
these 32-byte header fields can inject terminal control sequences
(color, title change, cursor moves) into the output of
"tar -tvf untrusted.tar" -- the most common command used to preview
untrusted archives. Fix: wrap user and group in quotearg_n, same
pattern as the filename on the next line.
Terminal escape injection via filename in timestamp warnings
When a member's mtime is far in the future or negative, "tar -xf" emits
a warning through WARNOPT(WARN_TIMESTAMP, ...) at src/extract.c:354 and
:371 that passes file_name raw to %s. WARN_TIMESTAMP is on by default.
An attacker who puts ANSI escapes in a member filename and sets a
bogus mtime gets terminal control-sequence injection during plain
"tar -xf" -- no verbose flag required. Fix: wrap file_name in
quotearg_colon at both call sites.
Inconsistent quoting of archive data in diagnostics
The two issues above share a root cause: there is no structural
enforcement that archive-derived strings must pass through quotearg
before reaching %s. Most call sites do it correctly; a few do not.
A full sweep of the roughly 450 WARN/ERROR/FATAL_ERROR call sites
would likely surface more. Consider adding a wrapper macro that bakes
in quoting for archive-sourced arguments, plus a CI grep check that
flags %s format specifiers paired with known header-field variables.
PoC archives and proposed patches available on request.
Please find a proposed patch attached to this email:
Quote archive-derived strings in diagnostic output
Two sites print strings taken directly from archive headers through a
%s format specifier without first passing them through quotearg. This
lets a crafted archive inject ANSI escape sequences (color changes,
terminal title rewrites, cursor manipulation) into a user's terminal
when running tar under default options.
1. src/list.c: the verbose listing emitted by "tar -tvf" prints the
uname and gname fields from the ustar header via %s. Those fields
are 32 bytes of attacker-controlled data. The filename on the next
line is already wrapped in quotearg; extend the same protection to
user and group.
2. src/extract.c: the WARN_TIMESTAMP diagnostics emitted during plain
"tar -xf" (for members with implausibly old or future mtimes) pass
file_name raw to %s. WARN_TIMESTAMP is on by default. Apply
quotearg_colon to match the pattern used elsewhere in the file
(e.g. the "Member name contains '..'" message).
Both issues reproduce on upstream tar 1.35 under default options. The
fixes follow the pattern already established in the surrounding code
and do not change output for archives whose header fields contain
only printable characters.
---
src/extract.c | 4 ++--
src/list.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
--- a/src/list.c
+++ b/src/list.c
@@ -1276,7 +1276,8 @@
ugswidth = pad;
fprintf (stdlis, "%s %s/%s %*s %-*s",
- modes, user, group, ugswidth - pad + sizelen, size,
+ modes, quotearg_n (2, user), quotearg_n (3, group),
+ ugswidth - pad + sizelen, size,
datewidth, time_stamp);
fprintf (stdlis, " %s", quotearg (temp_name));
--- a/src/extract.c
+++ b/src/extract.c
@@ -352,7 +352,7 @@
if (t.tv_sec < 0)
WARNOPT (WARN_TIMESTAMP,
(0, 0, _("%s: implausibly old time stamp %s"),
- file_name, tartime (t, true)));
+ quotearg_colon (file_name), tartime (t, true)));
else if (timespec_cmp (volume_start_time, t) < 0)
{
struct timespec now;
@@ -370,7 +370,7 @@
}
WARNOPT (WARN_TIMESTAMP,
(0, 0, _("%s: time stamp %s is %s s in the future"),
- file_name, tartime (t, true), code_timespec (diff, buf)));
+ quotearg_colon (file_name), tartime (t, true), code_timespec (diff, buf)));
}
}
}
