[PATCH v3 00/15] Fix segfaults on settings given without a value

["Thomas Bachem via GitGitGadget" <gitgitgadget@gmail.com>]

A setting given without a value, as in "git -c foo.bar" or a bare "bar"
under "[foo]", is passed as NULL to the code that reads it. In these places
git does not check for that and segfaults:

 * rebase.missingCommitsCheck in "git rebase -i"
 * core.alternateRefsCommand and core.alternateRefsPrefixes in "git fetch",
   "git receive-pack" and "git rev-list --alternate-refs", in a repository
   with an alternate
 * the color slots of "git add -p", "git add -i" and the other interactive
   patch modes
 * fetch.bundleCreationToken in "git fetch" and "git clone --bundle-uri",
   with a creationToken bundle list
 * grep.patternType in "git grep", "git log" and "git show"
 * remote.*.tagOpt in "git fetch", "git remote" and "git status"
 * color.blame.repeatedLines and color.blame.highlightRecent in "git blame"
 * branch.*.mergeOptions in "git merge" on that branch
 * remotes.* in "git fetch" and "git push" of that remote group
 * safe.bareRepository in any bare repository that git discovers
 * bitmapPseudoMerge.*.pattern when writing a bitmap, as in "git repack
   -adb"
 * scalar.repo in "scalar reconfigure --all"
 * remote.*.url in nearly every command, when an includeIf
   "hasconfig:remote.*.url:..." section exists
 * mode, heuristic, uri and creationToken in a bundle list, in "git clone
   --bundle-uri" and "git fetch"
 * any key at all in "git config list --type=color" and "--type=expiry-date"

Changes since v2:

 * Where an invalid value is ignored, a missing one now is too, with a
   warning: remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch
   7). v2 rejected them (Phillip).

 * The test in patch 1 rebases onto HEAD~3 and checks the result with
   test_commit_message (Phillip).

These turned up in a check of every config callback and every caller of
repo_config_get_value(), and in runs of two dozen common commands with each
key that "git help --config" lists set without a value. There may be more in
code that neither method reached.

Most patches reject the missing value with config_error_nonbool(), as
ba176db511 (config: handle NULL value when parsing non-bools, 2023-12-07)
did for other settings. Where git ignores an invalid value, it now ignores a
missing one too, with a warning: rebase.missingCommitsCheck (patch 1),
remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch 7). The
exception is a bundle list, where patch 14 rejects a missing heuristic or
creationToken even though an invalid one is accepted, as Derrick would
rather make the bundle URI code stricter about malformed lists than more
lenient. A missing fetch.bundleCreationToken counts as 0 (patch 4), a remote
URL without a value matches no includeIf condition (patch 13), and "git
config list" leaves the key out (patch 15). The crash fixed in patch 14 can
also come from a server, through a bundle list it advertises to clients that
use bundle URIs. It is still a plain NULL dereference. Only patch 15 fixes a
regression, which came in with 2.54.0.

remote.*.followRemoteHEAD without a value crashes the same commands. This
series leaves it alone, as ch/remote-defer-follow-remote-head-validation in
'seen' already makes it safe, checking the value only where fetch uses it.

The series is still based on 3c60ef1e38 (The 17th batch, 2026-11-08) and
merges cleanly into 'master', 'next' and 'seen'.

Thomas Bachem (15):
  rebase -i: handle NULL value for rebase.missingCommitsCheck
  odb: handle NULL values for core.alternateRefs* config
  add-patch: handle NULL value for color slots
  bundle-uri: handle NULL value for fetch.bundleCreationToken
  grep: handle NULL value for grep.patternType
  remote: handle NULL value for remote.<name>.tagOpt
  blame: handle NULL values for color.blame.* config
  merge: handle NULL value for branch.<name>.mergeOptions
  remote: handle NULL value for remotes.<group>
  setup: handle NULL value for safe.bareRepository
  pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
  scalar: handle NULL value for scalar.repo
  config: handle NULL value for remote URLs in hasconfig includes
  bundle-uri: handle NULL values in bundle lists
  config: handle NULL values when listing colors and expiry dates

 add-patch.c                     |  2 +-
 builtin/blame.c                 |  7 +++++++
 builtin/config.c                |  4 ++--
 builtin/merge.c                 |  2 ++
 bundle-uri.c                    | 12 +++++++++++-
 config.c                        |  3 ++-
 grep.c                          |  2 ++
 odb.c                           |  4 ++--
 pseudo-merge.c                  |  4 ++++
 rebase-interactive.c            | 10 ++++++++--
 remote.c                        |  9 ++++++++-
 scalar.c                        |  5 ++++-
 setup.c                         |  2 ++
 t/t0035-safe-bare-repository.sh |  6 ++++++
 t/t1300-config.sh               | 32 ++++++++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh   | 12 ++++++++++++
 t/t3701-add-interactive.sh      | 12 ++++++++++++
 t/t5333-pseudo-merge-bitmaps.sh |  5 +++++
 t/t5505-remote.sh               |  7 +++++++
 t/t5506-remote-groups.sh        |  5 +++++
 t/t5558-clone-bundle-uri.sh     | 20 ++++++++++++++++++++
 t/t5618-alternate-refs.sh       |  9 +++++++++
 t/t5750-bundle-uri-parse.sh     | 16 ++++++++++++++++
 t/t7600-merge.sh                |  6 ++++++
 t/t7810-grep.sh                 |  5 +++++
 t/t8012-blame-colors.sh         | 12 ++++++++++++
 t/t9210-scalar.sh               |  5 +++++
 27 files changed, 207 insertions(+), 11 deletions(-)


base-commit: 3c60ef1e38ee80337025a1b908327f7879386329
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2297%2Fthomasbachem%2Fconfig-null-value-segfaults-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2297/thomasbachem/config-null-value-segfaults-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2297

Range-diff vs v2:

  1:  47224a9893 !  1:  1262222ccd rebase -i: handle NULL value for rebase.missingCommitsCheck
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i respects rebase.mi
      +	rebase_setup_and_clean missing-commit &&
      +	(
      +		set_fake_editor &&
     -+		FAKE_LINES="1 2 3 4" \
     -+			git -c rebase.missingCommitsCheck rebase -i --root 2>actual
     ++		FAKE_LINES=1 \
     ++			git -c rebase.missingCommitsCheck rebase -i HEAD~3 2>actual
      +	) &&
     -+	test D = $(git cat-file commit HEAD | sed -ne \$p) &&
     ++	test_commit_message HEAD -m C &&
      +	test_grep "missing value for option rebase.missingCommitsCheck" actual &&
      +	test_grep ! "commits may have been dropped" actual
      +'
  2:  9170c4162f =  2:  ec9d7fd183 odb: handle NULL values for core.alternateRefs* config
  3:  d3fd6290e0 =  3:  c071e79d11 add-patch: handle NULL value for color slots
  4:  a822cbb844 =  4:  8773aaef0b bundle-uri: handle NULL value for fetch.bundleCreationToken
  5:  6c2a671a92 =  5:  123f03f024 grep: handle NULL value for grep.patternType
  6:  d4b49d1363 !  6:  339590cd4b remote: handle NULL value for remote.<name>.tagOpt
     @@ Commit message
          "git remote" and "git status" segfault: handle_config() passes the
          NULL value to strcmp().
      
     -    Reject a missing value with config_error_nonbool(), as handle_config()
     -    already does for remote.<name>.url and remote.<name>.pushurl.
     +    The setting takes "--no-tags" or "--tags", and any other value is
     +    ignored. Ignore a missing value too, but warn about it.
      
     +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
          Assisted-by: Claude Fable 5.1
     +    Assisted-by: Claude Opus 5.5
          Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
      
       ## remote.c ##
     @@ remote.c: static int handle_config(const char *key, const char *value,
       		else
       			error(_("more than one uploadpack given, using the first"));
       	} else if (!strcmp(subkey, "tagopt")) {
     -+		if (!value)
     -+			return config_error_nonbool(key);
     ++		if (!value) {
     ++			warning(_("missing value for '%s'"), key);
     ++			return 0;
     ++		}
       		if (!strcmp(value, "--no-tags"))
       			remote->fetch_tags = -1;
       		else if (!strcmp(value, "--tags"))
     @@ t/t5505-remote.sh: test_expect_success 'remote information for the origin' '
       	)
       '
       
     -+test_expect_success 'tagOpt needs a value' '
     -+	test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
     ++test_expect_success 'tagOpt without a value only warns' '
     ++	git -C test -c remote.origin.tagOpt remote >actual 2>err &&
     ++	echo origin >expect &&
     ++	test_cmp expect actual &&
      +	test_grep "missing value for .remote.origin.tagopt." err
      +'
      +
  7:  25843b43bf !  7:  ad50c4b35b blame: handle NULL values for color.blame.* config
     @@ Commit message
          value makes "git blame" segfault: git_blame_config() passes the NULL
          value to strlen() or to parse_color_fields().
      
     -    Reject a missing value with config_error_nonbool(), as
     +    An invalid color.blame.repeatedLines gets a warning and is ignored.
     +    Treat a missing value the same way.
     +
     +    An invalid color.blame.highlightRecent makes "git blame" die, so
     +    reject a missing value there with config_error_nonbool(), as
          git_blame_config() already does for blame.date and blame.coloring.
      
     +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
          Assisted-by: Claude Fable 5.1
     +    Assisted-by: Claude Opus 5.5
          Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
      
       ## builtin/blame.c ##
     @@ builtin/blame.c: static int git_blame_config(const char *var, const char *value,
       		return 0;
       	}
       	if (!strcmp(var, "color.blame.repeatedlines")) {
     -+		if (!value)
     -+			return config_error_nonbool(var);
     ++		if (!value) {
     ++			warning(_("missing value for '%s'"),
     ++				"color.blame.repeatedLines");
     ++			return 0;
     ++		}
       		if (color_parse_mem(value, strlen(value), repeated_meta_color))
       			warning(_("invalid value for '%s': '%s'"),
       				"color.blame.repeatedLines", value);
     @@ t/t8012-blame-colors.sh: test_expect_success 'blame color by age: new code is di
       	test_grep qfunc colored
       '
       
     -+test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
     -+	test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
     -+	test_grep "missing value for .color.blame.repeatedlines." err &&
     ++test_expect_success 'color.blame.repeatedLines without a value only warns' '
     ++	git blame --color-lines hello.c >expect &&
     ++	git -c color.blame.repeatedLines blame --color-lines hello.c >actual 2>err &&
     ++	test_cmp expect actual &&
     ++	test_grep "missing value for .color.blame.repeatedLines." err
     ++'
     ++
     ++test_expect_success 'color.blame.highlightRecent needs a value' '
      +	test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
      +	test_grep "missing value for .color.blame.highlightrecent." err
      +'
  8:  d8681a8560 =  8:  90209dc754 merge: handle NULL value for branch.<name>.mergeOptions
  9:  51f8eba073 =  9:  f30ecf07b4 remote: handle NULL value for remotes.<group>
 10:  a5b6436943 = 10:  bb6eecd20c setup: handle NULL value for safe.bareRepository
 11:  1533ef9cc6 = 11:  510dc56c9b pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
 12:  6219e9fd7f = 12:  751fae34b2 scalar: handle NULL value for scalar.repo
 13:  080106d5ea = 13:  4d5d2ce04e config: handle NULL value for remote URLs in hasconfig includes
 14:  fb9e63541b = 14:  474aebd572 bundle-uri: handle NULL values in bundle lists
 15:  12f841e5ff = 15:  f6f3295ff5 config: handle NULL values when listing colors and expiry dates

-- 
gitgitgadget