Skip to content

Commit

Permalink
reboot-util: Add some basic validation on reboot arguments
Browse files Browse the repository at this point in the history
Let's only accept valid ASCII and put a size limit on reboot arguments.

(cherry picked from commit b7ad477)
  • Loading branch information
DaanDeMeyer authored and keszybz committed May 8, 2024
1 parent 819f3f0 commit dba7fd5
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/core/dbus-unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,7 @@ static int bus_unit_set_transient_property(
return bus_set_transient_emergency_action(u, name, &u->job_timeout_action, message, flags, error);

if (streq(name, "JobTimeoutRebootArgument"))
return bus_set_transient_string(u, name, &u->job_timeout_reboot_arg, message, flags, error);
return bus_set_transient_reboot_parameter(u, name, &u->job_timeout_reboot_arg, message, flags, error);

if (streq(name, "StartLimitIntervalUSec"))
return bus_set_transient_usec(u, name, &u->start_ratelimit.interval, message, flags, error);
Expand All @@ -2234,7 +2234,7 @@ static int bus_unit_set_transient_property(
return bus_set_transient_exit_status(u, name, &u->success_action_exit_status, message, flags, error);

if (streq(name, "RebootArgument"))
return bus_set_transient_string(u, name, &u->reboot_arg, message, flags, error);
return bus_set_transient_reboot_parameter(u, name, &u->reboot_arg, message, flags, error);

if (streq(name, "CollectMode"))
return bus_set_transient_collect_mode(u, name, &u->collect_mode, message, flags, error);
Expand Down
2 changes: 2 additions & 0 deletions src/core/dbus-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "escape.h"
#include "parse-util.h"
#include "path-util.h"
#include "reboot-util.h"
#include "unit-printf.h"
#include "user-util.h"
#include "unit.h"
Expand Down Expand Up @@ -39,6 +40,7 @@ static bool valid_user_group_name_or_id_relaxed(const char *u) {

BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(user_relaxed, valid_user_group_name_or_id_relaxed);
BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(path, path_is_absolute);
BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(reboot_parameter, reboot_parameter_is_valid);

int bus_set_transient_string(
Unit *u,
Expand Down
1 change: 1 addition & 0 deletions src/core/dbus-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ int bus_set_transient_mode_t(Unit *u, const char *name, mode_t *p, sd_bus_messag
int bus_set_transient_unsigned(Unit *u, const char *name, unsigned *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
int bus_set_transient_user_relaxed(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
int bus_set_transient_path(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
int bus_set_transient_reboot_parameter(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
int bus_set_transient_string(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
int bus_set_transient_bool(Unit *u, const char *name, bool *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
int bus_set_transient_tristate(Unit *u, const char *name, int *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error);
Expand Down
4 changes: 2 additions & 2 deletions src/core/load-fragment-gperf.gperf.in
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ Unit.IgnoreOnSnapshot, config_parse_warn_compat,
Unit.JobTimeoutSec, config_parse_job_timeout_sec, 0, 0
Unit.JobRunningTimeoutSec, config_parse_job_running_timeout_sec, 0, 0
Unit.JobTimeoutAction, config_parse_emergency_action, 0, offsetof(Unit, job_timeout_action)
Unit.JobTimeoutRebootArgument, config_parse_unit_string_printf, 0, offsetof(Unit, job_timeout_reboot_arg)
Unit.JobTimeoutRebootArgument, config_parse_reboot_parameter, 0, offsetof(Unit, job_timeout_reboot_arg)
Unit.StartLimitIntervalSec, config_parse_sec, 0, offsetof(Unit, start_ratelimit.interval)
{# The following is a legacy alias name for compatibility #}
Unit.StartLimitInterval, config_parse_sec, 0, offsetof(Unit, start_ratelimit.interval)
Expand All @@ -335,7 +335,7 @@ Unit.FailureAction, config_parse_emergency_action,
Unit.SuccessAction, config_parse_emergency_action, 0, offsetof(Unit, success_action)
Unit.FailureActionExitStatus, config_parse_exit_status, 0, offsetof(Unit, failure_action_exit_status)
Unit.SuccessActionExitStatus, config_parse_exit_status, 0, offsetof(Unit, success_action_exit_status)
Unit.RebootArgument, config_parse_unit_string_printf, 0, offsetof(Unit, reboot_arg)
Unit.RebootArgument, config_parse_reboot_parameter, 0, offsetof(Unit, reboot_arg)
Unit.ConditionPathExists, config_parse_unit_condition_path, CONDITION_PATH_EXISTS, offsetof(Unit, conditions)
Unit.ConditionPathExistsGlob, config_parse_unit_condition_path, CONDITION_PATH_EXISTS_GLOB, offsetof(Unit, conditions)
Unit.ConditionPathIsDirectory, config_parse_unit_condition_path, CONDITION_PATH_IS_DIRECTORY, offsetof(Unit, conditions)
Expand Down
35 changes: 35 additions & 0 deletions src/core/load-fragment.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "pcre2-util.h"
#include "percent-util.h"
#include "process-util.h"
#include "reboot-util.h"
#include "seccomp-util.h"
#include "securebits-util.h"
#include "selinux-util.h"
Expand Down Expand Up @@ -361,6 +362,40 @@ int config_parse_unit_string_printf(
return config_parse_string(unit, filename, line, section, section_line, lvalue, ltype, k, data, userdata);
}

int config_parse_reboot_parameter(
const char *unit,
const char *filename,
unsigned line,
const char *section,
unsigned section_line,
const char *lvalue,
int ltype,
const char *rvalue,
void *data,
void *userdata) {

_cleanup_free_ char *k = NULL;
const Unit *u = ASSERT_PTR(userdata);
int r;

assert(filename);
assert(line);
assert(rvalue);

r = unit_full_printf(u, rvalue, &k);
if (r < 0) {
log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to resolve unit specifiers in '%s', ignoring: %m", rvalue);
return 0;
}

if (!reboot_parameter_is_valid(k)) {
log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid reboot parameter '%s', ignoring.", k);
return 0;
}

return config_parse_string(unit, filename, line, section, section_line, lvalue, ltype, k, data, userdata);
}

int config_parse_unit_strv_printf(
const char *unit,
const char *filename,
Expand Down
1 change: 1 addition & 0 deletions src/core/load-fragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ void unit_dump_config_items(FILE *f);
CONFIG_PARSER_PROTOTYPE(config_parse_unit_deps);
CONFIG_PARSER_PROTOTYPE(config_parse_obsolete_unit_deps);
CONFIG_PARSER_PROTOTYPE(config_parse_unit_string_printf);
CONFIG_PARSER_PROTOTYPE(config_parse_reboot_parameter);
CONFIG_PARSER_PROTOTYPE(config_parse_unit_strv_printf);
CONFIG_PARSER_PROTOTYPE(config_parse_unit_path_printf);
CONFIG_PARSER_PROTOTYPE(config_parse_colon_separated_paths);
Expand Down
3 changes: 3 additions & 0 deletions src/login/logind-dbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,9 @@ static int method_set_reboot_parameter(
if (r < 0)
return r;

if (!reboot_parameter_is_valid(arg))
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid reboot parameter '%s'.", arg);

r = detect_container();
if (r < 0)
return r;
Expand Down
10 changes: 10 additions & 0 deletions src/shared/reboot-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@
#include "reboot-util.h"
#include "string-util.h"
#include "umask-util.h"
#include "utf8.h"
#include "virt.h"

bool reboot_parameter_is_valid(const char *parameter) {
assert(parameter);

return ascii_is_valid(parameter) && strlen(parameter) <= NAME_MAX;
}

int update_reboot_parameter_and_warn(const char *parameter, bool keep) {
int r;

Expand All @@ -42,6 +49,9 @@ int update_reboot_parameter_and_warn(const char *parameter, bool keep) {
return 0;
}

if (!reboot_parameter_is_valid(parameter))
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid reboot parameter '%s'.", parameter);

WITH_UMASK(0022) {
r = write_string_file("/run/systemd/reboot-param", parameter,
WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC);
Expand Down
1 change: 1 addition & 0 deletions src/shared/reboot-util.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#pragma once

bool reboot_parameter_is_valid(const char *parameter);
int update_reboot_parameter_and_warn(const char *parameter, bool keep);

typedef enum RebootFlags {
Expand Down

0 comments on commit dba7fd5

Please sign in to comment.