Repository navigation
pr-2297/thomasbachem/config-null-value-segfaults-v2
pr-2297/thomasbachem/config-null-value-segfaults-v2: Fix segfaults on settings given without a value
tagged this
10 Nov 07:36
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 v1: * Patch 1 no longer rejects a rebase.missingCommitsCheck without a value. It warns and falls back to "ignore", as for a value that git does not recognize (Phillip, Junio). * Rebased onto master, where iw/connected-deepen-fetch-no-alternate-refs now adds tests at the end of t5618, as patch 2 does. 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. Four accept it without an error: a missing rebase.missingCommitsCheck warns and falls back to "ignore" like a value that git does not recognize (patch 1), 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 now based on 3c60ef1e38 (The 17th batch, 2026-11-08) and merges cleanly into 'next' and 'seen' too. 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 | 4 ++++ 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 | 7 ++++++- 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 | 5 +++++ 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 | 7 +++++++ t/t9210-scalar.sh | 5 +++++ 27 files changed, 195 insertions(+), 11 deletions(-) base-commit: 3c60ef1e38ee80337025a1b908327f7879386329 Submitted-As: https://lore.kernel.org/git/pull.2297.v2.git.1794296205.gitgitgadget@gmail.com In-Reply-To: https://lore.kernel.org/git/pull.2297.git.1794040967.gitgitgadget@gmail.com