mirror of
https://github.com/systemd/systemd
synced 2024-07-22 02:34:54 +00:00
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.
This commit is contained in:
parent
6b42227edb
commit
a88f9dbae2
|
@ -79,6 +79,13 @@ void* memdup_suffix0(const void *p, size_t l); /* We can't use _alloc_() here, s
|
|||
memcpy_safe(_q_, p, _l_); \
|
||||
})
|
||||
|
||||
static inline void unsetp(void *p) {
|
||||
/* A trivial "destructor" that can be used in cases where we want to
|
||||
* unset a pointer from a _cleanup_ function. */
|
||||
|
||||
*(void**)p = NULL;
|
||||
}
|
||||
|
||||
static inline void freep(void *p) {
|
||||
*(void**)p = mfree(*(void**) p);
|
||||
}
|
||||
|
|
|
@ -22,7 +22,7 @@ int kill_unit(int argc, char *argv[], void *userdata) {
|
|||
arg_kill_who = "all";
|
||||
|
||||
/* --fail was specified */
|
||||
if (streq(arg_job_mode, "fail"))
|
||||
if (streq(arg_job_mode(), "fail"))
|
||||
kill_who = strjoina(arg_kill_who, "-fail");
|
||||
|
||||
r = expand_unit_names(bus, strv_skip(argv, 1), NULL, &names, NULL);
|
||||
|
|
|
@ -306,7 +306,7 @@ int start_unit(int argc, char *argv[], void *userdata) {
|
|||
/* A command in style of "systemctl start <unit1> <unit2> …", "sysemctl stop <unit1> <unit2> …" and so on */
|
||||
method = verb_to_method(argv[0]);
|
||||
job_type = verb_to_job_type(argv[0]);
|
||||
mode = arg_job_mode;
|
||||
mode = arg_job_mode();
|
||||
} else
|
||||
method = job_type = mode = NULL;
|
||||
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -49,7 +49,7 @@ extern char **arg_states;
|
|||
extern char **arg_properties;
|
||||
extern bool arg_all;
|
||||
extern enum dependency arg_dependency;
|
||||
extern const char *arg_job_mode;
|
||||
extern const char *_arg_job_mode;
|
||||
extern UnitFileScope arg_scope;
|
||||
extern bool arg_wait;
|
||||
extern bool arg_no_block;
|
||||
|
@ -96,4 +96,8 @@ extern bool arg_read_only;
|
|||
extern bool arg_mkdir;
|
||||
extern bool arg_marked;
|
||||
|
||||
static inline const char* arg_job_mode(void) {
|
||||
return _arg_job_mode ?: "replace";
|
||||
}
|
||||
|
||||
int systemctl_dispatch_parse_argv(int argc, char *argv[]);
|
||||
|
|
BIN
test/fuzz/fuzz-systemctl-parse-argv/oss-fuzz-31714
Normal file
BIN
test/fuzz/fuzz-systemctl-parse-argv/oss-fuzz-31714
Normal file
Binary file not shown.
Loading…
Reference in a new issue