On 11/04/2021 11:55, Miriam Rubio wrote:
From: Tanushree Tumane <tanushreetumane@xxxxxxxxx>
Reimplement the `bisect_run()` shell function
in C and also add `--bisect-run` subcommand to
`git bisect--helper` to call it from git-bisect.sh.
Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
Signed-off-by: Tanushree Tumane <tanushreetumane@xxxxxxxxx>
Signed-off-by: Miriam Rubio <mirucam@xxxxxxxxx>
---
builtin/bisect--helper.c | 83 ++++++++++++++++++++++++++++++++++++++++
git-bisect.sh | 62 +-----------------------------
2 files changed, 84 insertions(+), 61 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 71963979b1..dc87fc7dd0 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -18,6 +18,7 @@ static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_head_name, "head-name")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-reset [<commit>]"),
@@ -31,6 +32,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-replay <filename>"),
N_("git bisect--helper --bisect-skip [(<rev>|<range>)...]"),
N_("git bisect--helper --bisect-visualize"),
+ N_("git bisect--helper --bisect-run <cmd>..."),
NULL
};
@@ -1073,6 +1075,78 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a
return res;
}
+static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
+{
+ int res = BISECT_OK;
+ struct strbuf command = STRBUF_INIT;
+ struct strvec args = STRVEC_INIT;
+ struct strvec run_args = STRVEC_INIT;
+ int exit = 0;
+ int temporary_stdout_fd, saved_stdout;
+
+ if (bisect_next_check(terms, NULL))
+ return BISECT_FAILED;
+
+ if (argc)
+ sq_quote_argv(&command, argv);
+ else
+ return BISECT_FAILED;
+
+ run_args.v[0] = xstrdup(command.buf);
+ run_args.nr = 1;
AFAIUI manipulating the strvec directly like this means that we will
violate the promise that strvec.v is always NULL-terminated. It's
probably safer to call 'strvec_push(run_args, command.buf)' instead of
manipulating v and nr?
Violating the NULL-termination promise a problem because... (continued
below)
+
+ while (1) {
+ strvec_clear(&args);
+ exit = 1;
+
+ printf(_("running %s"), command.buf);
+ res = run_command_v_opt(run_args.v, RUN_USING_SHELL);
run_command_v_opt() implicitly expects a NULL-terminated list of
strings. It's not documented in run_command_v_opt()'s comments, however
run_command_v_opt() does explain that it's a wrapper around
start_command(), which uses child_process, and child_process.argv is
documented to require a NULL-terminated list.
If argv is not NULL-terminated, we hit a buffer overflow read in
prepare_shell_cmd(), which can be reproduced by running something like:
make CC=clang-11 SANITIZE=address COPTS="-Og -g" GIT_TEST_OPTS=-v
T=t6030-bisect-porcelain.sh test
which results in ASAN reporting this error:
==2116==ERROR: AddressSanitizer: global-buffer-overflow on address
0x000001a51e28 at pc 0x0000009c6c1f bp 0x7ffcf0f60670 sp 0x7ffcf0f60668
READ of size 8 at 0x000001a51e28 thread T0
#0 0x9c6c1e in prepare_shell_cmd run-command.c:284:8
#1 0x9c6c1e in prepare_cmd run-command.c:419:3
#2 0x9c6c1e in start_command run-command.c:753:6
#3 0x9c7d35 in run_command run-command.c:1015:9
#4 0x9c800c in run_command_v_opt_cd_env_tr2 run-command.c:1051:9
#5 0x9c800c in run_command_v_opt_cd_env run-command.c:1033:9
#6 0x9c800c in run_command_v_opt run-command.c:1023:9
#7 0x4e5b60 in bisect_run builtin/bisect--helper.c:1102:9
#8 0x4e5b60 in cmd_bisect__helper builtin/bisect--helper.c:1252:9
#9 0x4ce8fe in run_builtin git.c:461:11
#10 0x4ccbc8 in handle_builtin git.c:718:3
#11 0x4cb0cc in run_argv git.c:785:4
#12 0x4cb0cc in cmd_main git.c:916:19
#13 0x6beded in main common-main.c:52:11
#14 0x7f28636f7349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#15 0x420769 in _start ../sysdeps/x86_64/start.S:120
0x000001a51e28 is located 56 bytes to the left of global variable
'config_update_recurse_submodules' defined in 'submodule.c:26:12'
(0x1a51e60) of size 4
0x000001a51e28 is located 0 bytes to the right of global variable
'empty_strvec' defined in 'strvec.c:5:13' (0x1a51e20) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow run-command.c:284:8 in
prepare_shell_cmd
[... snip the rest ...]
