diff options
author | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2021-05-31 11:23:20 +0200 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2021-05-31 19:29:07 +0200 |
commit | a88f9dbae2cbd2af6016e7f4dab5b7a0771491c6 (patch) | |
tree | 346d30a31c2b1ebdce8e1d10f8c4b811fbdc221e /src/systemctl/systemctl.c | |
parent | 6b42227edb78193294d8096d39751b5fa45985e7 (diff) | |
download | systemd-a88f9dbae2cbd2af6016e7f4dab5b7a0771491c6.tar.gz |
systemctl: unset const char* arguments in static destructors
When fuzzing, the following happens:
- we parse 'data' and produce an argv array,
- one of the items in argv is assigned to arg_host,
- the argv array is subsequently freed by strv_freep(), and arg_host has a dangling symlink.
In normal use, argv is static, so arg_host can never become a dangling pointer.
In fuzz-systemctl-parse-argv, if we repeatedly parse the same array, we
have some dangling pointers while we're in the middle of parsing. If we parse
the same array a second time, at the end all the dangling pointers will have been
replaced again. But for a short time, if parsing one of the arguments uses another
argument, we would use a dangling pointer.
Such a case occurs when we have --host=… --boot-loader-entry=help. The latter calls
acquire_bus() which uses arg_host.
I'm not particularly happy with making the code more complicated just for
fuzzing, but I think it's better to resolve this, even if the issue cannot
occur in normal invocations, than to deal with fuzzer reports.
Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=31714.
Diffstat (limited to 'src/systemctl/systemctl.c')
-rw-r--r-- | src/systemctl/systemctl.c | 15 |
1 files changed, 10 insertions, 5 deletions
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index a4100bd554..e37569ab7f 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -65,7 +65,7 @@ char **arg_states = NULL; char **arg_properties = NULL; bool arg_all = false; enum dependency arg_dependency = DEPENDENCY_FORWARD; -const char *arg_job_mode = "replace"; +const char *_arg_job_mode = NULL; UnitFileScope arg_scope = UNIT_FILE_SYSTEM; bool arg_wait = false; bool arg_no_block = false; @@ -115,8 +115,13 @@ bool arg_marked = false; STATIC_DESTRUCTOR_REGISTER(arg_types, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_states, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_properties, strv_freep); +STATIC_DESTRUCTOR_REGISTER(_arg_job_mode, unsetp); STATIC_DESTRUCTOR_REGISTER(arg_wall, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_kill_who, unsetp); STATIC_DESTRUCTOR_REGISTER(arg_root, freep); +STATIC_DESTRUCTOR_REGISTER(arg_reboot_argument, unsetp); +STATIC_DESTRUCTOR_REGISTER(arg_host, unsetp); +STATIC_DESTRUCTOR_REGISTER(arg_boot_loader_entry, unsetp); STATIC_DESTRUCTOR_REGISTER(arg_clean_what, strv_freep); static int systemctl_help(void) { @@ -598,19 +603,19 @@ static int systemctl_parse_argv(int argc, char *argv[]) { break; case ARG_JOB_MODE: - arg_job_mode = optarg; + _arg_job_mode = optarg; break; case ARG_FAIL: - arg_job_mode = "fail"; + _arg_job_mode = "fail"; break; case ARG_IRREVERSIBLE: - arg_job_mode = "replace-irreversibly"; + _arg_job_mode = "replace-irreversibly"; break; case ARG_IGNORE_DEPENDENCIES: - arg_job_mode = "ignore-dependencies"; + _arg_job_mode = "ignore-dependencies"; break; case ARG_USER: |