What did you do before the bug happened? (Steps to reproduce your issue)
git init repo && cd repo
printf 'one\ntwo\nthree\n' >f.txt
git add f.txt && git commit -m init
printf 'one\nTWO\nthree\n' >f.txt
git stash
git stash show --src-prefix=a/ --dst-prefix=b/
What did you expect to happen? (Expected behavior)
The first line of the patch should use the prefixes I asked for:
diff --git a/f.txt b/f.txt
What happened instead? (Actual behavior)
The prefixes are replaced by fragments of unrelated heap data, and the
value changes between runs of the same command:
$ git stash show --src-prefix=a/ --dst-prefix=b/ | head -1
diff --git Uf.txt Uf.txt
$ git stash show --src-prefix=a/ --dst-prefix=b/ | head -1
diff --git Vf.txt Vf.txt
On other versions the garbage is recognisable as pieces of other
strings live in the process -- "ributes" (from "attributes"),
"bjectmode" (from "objectmode"), "4c/" -- which is what suggests a
use-after-free rather than an off-by-one.
What's different between what you expected and what actually happened?
Scope, from testing across released versions.
"git diff --src-prefix=a/ --dst-prefix=b/" is correct on every version
I tried. Only "stash show" is affected. First line of the patch from
"git stash show --src-prefix=a/ --dst-prefix=b/":
2.49.1 diff --git a/f.txt b/f.txt (correct)
2.52.0 diff --git ributesf.txt 4c/f.txt
2.53.0 diff --git Uf.txt Uf.txt
2.54.0 diff --git 4c/f.txt bjectmodef.txt
The 2.53.0 output varies between invocations; the others were stable
within a single container but differ from each other.
Also unaffected: "git stash show -p" with no prefix flags, and
"git stash show -p --no-ext-diff --no-textconv".
Anything else you want to add:
Suspected cause. 3ea35c64b ("stash: tell setup_revisions() to free our
allocated strings", merged in jk/setup-revisions-freefix) added
struct setup_revision_opt opt = { .free_removed_argv_elements = 1 };
to show_stash(). v2.51.0 does not contain that commit; v2.52.0 does,
which matches the bisect above.
--src-prefix and --dst-prefix are parsed by OPT_STRING_F in diff.c:
OPT_STRING_F(0, "src-prefix", &options->a_prefix, N_("<prefix>"),
N_("show the given source prefix instead of \"a/\""),
PARSE_OPT_NONEG),
parse-options stores the pointer into the argv element rather than
copying it, so options->a_prefix points into the "--src-prefix=a/"
string itself. Once setup_revisions() is told it may free the argv
elements it consumes, that string is freed while a_prefix still
references it, and the dangling pointer is read later when the diff
header is emitted.
If that reading is right, the same hazard would apply to any diff
option parsed with OPT_STRING* into a struct diff_options field, not
only these two -- "stash show" is simply the caller that now opts in
to the freeing.
How I ran into it: a tool that passes --src-prefix=a/ --dst-prefix=b/
explicitly so it can parse the resulting patch without being affected
by a user's diff.noprefix or diff.mnemonicPrefix configuration. That
is a fairly common pattern for programs consuming git's diff output
(lint-staged does the same), so the corrupted paths surface as
unparseable filenames rather than as an obvious crash.
I could not find an existing report for this.
[System Info]
git version 2.53.0 (Debian). Reproduced identically on the
alpine/git 2.52.0 and 2.54.0 images; not reproducible on 2.49.1.
Thanks,
Nicolas Le Cam