@matheusbsbs / matheus-bagliano / commits / ba176db511

config: handle NULL value when parsing non-bools

When the config parser sees an "implicit" bool like: [core] someVariable it passes NULL to the config callback. Any callback code which expects a string must check for NULL. This usually happens via helpers like git_config_string(), etc, but some custom code forgets to do so and will segfault. These are all fairly vanilla cases where the solution is just the usual pattern of: if (!value) return config_error_nonbool(var); though note that in a few cases we have to split initializers like: int some_var = initializer(); into: int some_var; if (!value) return config_error_nonbool(var); some_var = initializer(); There are still some broken instances after this patch, which I'll address on their own in individual patches after this one. Reported-by: Carlos Andrés Ramírez Cataño <antaigroupltda@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 7, 2023 at 02:11 UTC ba176db511b3438738a4aeb98e574310e697ff5f

11 files changed +47 -5

builtin/blame.c

@@ -748,6 +748,8 @@ static int git_blame_config(const char *var, const char *value,

748 }

749

750 if (!strcmp(var, "blame.coloring")) {

751 + if (!value)

752 + return config_error_nonbool(var);

753 if (!strcmp(value, "repeatedLines")) {

754 coloring_mode |= OUTPUT_COLOR_LINE;

755 } else if (!strcmp(value, "highlightRecent")) {

builtin/checkout.c

@@ -1202,6 +1202,8 @@ static int git_checkout_config(const char *var, const char *value,

1202 struct checkout_opts *opts = cb;

1203

1204 if (!strcmp(var, "diff.ignoresubmodules")) {

1205 + if (!value)

1206 + return config_error_nonbool(var);

1207 handle_ignore_submodules_arg(&opts->diff_options, value);

1208 return 0;

1209 }

builtin/clone.c

@@ -791,6 +791,8 @@ static int git_clone_config(const char *k, const char *v,

791 const struct config_context *ctx, void *cb)

792 {

793 if (!strcmp(k, "clone.defaultremotename")) {

794 + if (!v)

795 + return config_error_nonbool(k);

796 free(remote_name);

797 remote_name = xstrdup(v);

798 }

builtin/log.c

+4 -1

@@ -594,8 +594,11 @@ static int git_log_config(const char *var, const char *value,

594 decoration_style = 0; /* maybe warn? */

595 return 0;

596 }

597 - if (!strcmp(var, "log.diffmerges"))

597 + if (!strcmp(var, "log.diffmerges")) {

598 + if (!value)

599 + return config_error_nonbool(var);

600 return diff_merges_config(value);

601 + }

602 if (!strcmp(var, "log.showroot")) {

603 default_show_root = git_config_bool(var, value);

604 return 0;

builtin/pack-objects.c

+5 -1

@@ -3204,7 +3204,7 @@ static int git_pack_config(const char *k, const char *v,

3204 return 0;

3205 }

3206 if (!strcmp(k, "uploadpack.blobpackfileuri")) {

3207 - struct configured_exclusion *ex = xmalloc(sizeof(*ex));

3207 + struct configured_exclusion *ex;

3208 const char *oid_end, *pack_end;

3209 /*

3210 * Stores the pack hash. This is not a true object ID, but is

@@ -3212,6 +3212,10 @@ static int git_pack_config(const char *k, const char *v,

3212 */

3213 struct object_id pack_hash;

3214

3215 + if (!v)

3216 + return config_error_nonbool(k);

3217 +

3218 + ex = xmalloc(sizeof(*ex));

3219 if (parse_oid_hex(v, &ex->e.oid, &oid_end) ||

3220 *oid_end != ' ' ||

3221 parse_oid_hex(oid_end + 1, &pack_hash, &pack_end) ||

compat/mingw.c

@@ -255,6 +255,8 @@ int mingw_core_config(const char *var, const char *value,

255 }

256

257 if (!strcmp(var, "core.unsetenvvars")) {

258 + if (!value)

259 + return config_error_nonbool(var);

260 free(unset_environment_variables);

261 unset_environment_variables = xstrdup(value);

262 return 0;

config.c

@@ -1386,6 +1386,8 @@ static int git_default_core_config(const char *var, const char *value,

1386 return 0;

1387 }

1388 if (!strcmp(var, "core.checkstat")) {

1389 + if (!value)

1390 + return config_error_nonbool(var);

1391 if (!strcasecmp(value, "default"))

1392 check_stat = 1;

1393 else if (!strcasecmp(value, "minimal"))

@@ -1547,11 +1549,15 @@ static int git_default_core_config(const char *var, const char *value,

1549 }

1550

1551 if (!strcmp(var, "core.checkroundtripencoding")) {

1552 + if (!value)

1553 + return config_error_nonbool(var);

1554 check_roundtrip_encoding = xstrdup(value);

1555 return 0;

1556 }

1557

1558 if (!strcmp(var, "core.notesref")) {

1559 + if (!value)

1560 + return config_error_nonbool(var);

1561 notes_ref_name = xstrdup(value);

1562 return 0;

1563 }

@@ -1619,6 +1625,8 @@ static int git_default_core_config(const char *var, const char *value,

1625 }

1626

1627 if (!strcmp(var, "core.createobject")) {

1628 + if (!value)

1629 + return config_error_nonbool(var);

1630 if (!strcmp(value, "rename"))

1631 object_creation_mode = OBJECT_CREATION_USES_RENAMES;

1632 else if (!strcmp(value, "link"))

diff.c

+16 -3

@@ -372,7 +372,10 @@ int git_diff_ui_config(const char *var, const char *value,

372 return 0;

373 }

374 if (!strcmp(var, "diff.colormovedws")) {

375 - unsigned cm = parse_color_moved_ws(value);

375 + unsigned cm;

376 + if (!value)

377 + return config_error_nonbool(var);

378 + cm = parse_color_moved_ws(value);

379 if (cm & COLOR_MOVED_WS_ERROR)

380 return -1;

381 diff_color_moved_ws_default = cm;

@@ -426,10 +429,15 @@ int git_diff_ui_config(const char *var, const char *value,

429 if (!strcmp(var, "diff.orderfile"))

430 return git_config_pathname(&diff_order_file_cfg, var, value);

431

429 - if (!strcmp(var, "diff.ignoresubmodules"))

432 + if (!strcmp(var, "diff.ignoresubmodules")) {

433 + if (!value)

434 + return config_error_nonbool(var);

435 handle_ignore_submodules_arg(&default_diff_options, value);

436 + }

437

438 if (!strcmp(var, "diff.submodule")) {

439 + if (!value)

440 + return config_error_nonbool(var);

441 if (parse_submodule_params(&default_diff_options, value))

442 warning(_("Unknown value for 'diff.submodule' config variable: '%s'"),

443 value);

@@ -473,7 +481,10 @@ int git_diff_basic_config(const char *var, const char *value,

481 }

482

483 if (!strcmp(var, "diff.wserrorhighlight")) {

476 - int val = parse_ws_error_highlight(value);

484 + int val;

485 + if (!value)

486 + return config_error_nonbool(var);

487 + val = parse_ws_error_highlight(value);

488 if (val < 0)

489 return -1;

490 ws_error_highlight_default = val;

@@ -490,6 +501,8 @@ int git_diff_basic_config(const char *var, const char *value,

501

502 if (!strcmp(var, "diff.dirstat")) {

503 struct strbuf errmsg = STRBUF_INIT;

504 + if (!value)

505 + return config_error_nonbool(var);

506 default_diff_options.dirstat_permille = diff_dirstat_permille_default;

507 if (parse_dirstat_params(&default_diff_options, value, &errmsg))

508 warning(_("Found errors in 'diff.dirstat' config variable:\n%s"),

mailinfo.c

@@ -1253,6 +1253,8 @@ static int git_mailinfo_config(const char *var, const char *value,

1253 return 0;

1254 }

1255 if (!strcmp(var, "mailinfo.quotedcr")) {

1256 + if (!value)

1257 + return config_error_nonbool(var);

1258 if (mailinfo_parse_quoted_cr_action(value, &mi->quoted_cr) != 0)

1259 return error(_("bad action '%s' for '%s'"), value, var);

1260 return 0;

notes-utils.c

@@ -112,6 +112,8 @@ static int notes_rewrite_config(const char *k, const char *v,

112 }

113 return 0;

114 } else if (!c->refs_from_env && !strcmp(k, "notes.rewriteref")) {

115 + if (!v)

116 + return config_error_nonbool(k);

117 /* note that a refs/ prefix is implied in the

118 * underlying for_each_glob_ref */

119 if (starts_with(v, "refs/notes/"))

trailer.c

@@ -507,6 +507,8 @@ static int git_trailer_default_config(const char *conf_key, const char *value,

507 warning(_("unknown value '%s' for key '%s'"),

508 value, conf_key);

509 } else if (!strcmp(trailer_item, "separators")) {

510 + if (!value)

511 + return config_error_nonbool(conf_key);

512 separators = xstrdup(value);

513 }

514 }