[PATCH v2] config.c: NULL check when reading protected config

["Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>]

From: Glen Choo <chooglen@google.com>

In read_protected_config(), check whether each file name is NULL before
attempting to read it, and add a BUG() call to
git_config_from_file_with_options() to make this error easier to catch
in the future.

The NULL checks mirror what do_git_config_sequence() does (which
read_protected_config() is modeled after). Without these NULL checks,
multiple tests fail with "make SANITIZE=address", e.g. in the final test
of t4010, xdg_config is NULL causing us to call fopen(NULL).

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Glen Choo <chooglen@google.com>
---
    config.c: NULL check when reading protected config
    
    This fixes the SANITIZE=address failure on master, That was introduced
    by gc/bare-repo-discovery. Thanks again to Ævar for the original report
    [1] and for proposing a way to catch this in CI [2].
    
    Changes in v2:
    
     * Fix typo
     * Add BUG() to git_config_from_file_with_options()
    
    [1]
    https://lore.kernel.org/git/220725.861qu9oxl4.gmgdl@evledraar.gmail.com
    [2]
    https://lore.kernel.org/git/patch-1.1-e48b6853dd5-20220726T110716Z-avarab@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1299%2Fchooglen%2Fconfig%2Ffix-sanitize-address-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1299/chooglen/config/fix-sanitize-address-v2
Pull-Request: https://github.com/git/git/pull/1299

Range-diff vs v1:

 1:  17b4a489c69 ! 1:  ba51078418a config.c: NULL check when reading protected config
     @@ Commit message
          config.c: NULL check when reading protected config
      
          In read_protected_config(), check whether each file name is NULL before
     -    attempting to read it. This mirrors do_git_config_sequence() (which
     -    read_protected_config() is modelled after).
     +    attempting to read it, and add a BUG() call to
     +    git_config_from_file_with_options() to make this error easier to catch
     +    in the future.
      
     -    Without these NULL checks,
     -
     -     make SANITIZE=address test T=t0410*.sh
     -
     -    fails because xdg_config is NULL, causing us to call fopen(NULL).
     +    The NULL checks mirror what do_git_config_sequence() does (which
     +    read_protected_config() is modeled after). Without these NULL checks,
     +    multiple tests fail with "make SANITIZE=address", e.g. in the final test
     +    of t4010, xdg_config is NULL causing us to call fopen(NULL).
      
          Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
          Signed-off-by: Glen Choo <chooglen@google.com>
      
       ## config.c ##
     +@@ config.c: int git_config_from_file_with_options(config_fn_t fn, const char *filename,
     + 	int ret = -1;
     + 	FILE *f;
     + 
     ++	if (!filename)
     ++		BUG("filename cannot be NULL");
     + 	f = fopen_or_warn(filename, "r");
     + 	if (f) {
     + 		ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename,
      @@ config.c: static void read_protected_config(void)
       	system_config = git_system_config();
       	git_global_config(&user_config, &xdg_config);


 config.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 015bec360f5..e8ebef77d5c 100644
--- a/config.c
+++ b/config.c
@@ -1979,6 +1979,8 @@ int git_config_from_file_with_options(config_fn_t fn, const char *filename,
 	int ret = -1;
 	FILE *f;
 
+	if (!filename)
+		BUG("filename cannot be NULL");
 	f = fopen_or_warn(filename, "r");
 	if (f) {
 		ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename,
@@ -2645,9 +2647,12 @@ static void read_protected_config(void)
 	system_config = git_system_config();
 	git_global_config(&user_config, &xdg_config);
 
-	git_configset_add_file(&protected_config, system_config);
-	git_configset_add_file(&protected_config, xdg_config);
-	git_configset_add_file(&protected_config, user_config);
+	if (system_config)
+		git_configset_add_file(&protected_config, system_config);
+	if (xdg_config)
+		git_configset_add_file(&protected_config, xdg_config);
+	if (user_config)
+		git_configset_add_file(&protected_config, user_config);
 	git_configset_add_parameters(&protected_config);
 
 	free(system_config);

base-commit: 6a475b71f8c4ce708d69fdc9317aefbde3769e25
-- 
gitgitgadget