Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

some safety tweaks to conf-parser.[ch] #22458

Merged
merged 7 commits into from Feb 9, 2022
Merged

some safety tweaks to conf-parser.[ch] #22458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d17d69
conf-parse: add generic config_parse_safe_string() helper
poettering Feb 9, 2022
deec0b6
conf-parser: use _cleanup_free_ where appropriate
poettering Feb 9, 2022
73a4ac8
conf-parser: minor refactorings/modernizations
poettering Feb 9, 2022
cec7f09
conf-parser: refuse section headers with control characters early, ju…
poettering Feb 9, 2022
0b95409
conf-parser: update config_item_*_lookup() to follow modern coding style
poettering Feb 9, 2022
a99a852
tree-wide: use config_parse_safe_string() at various places
poettering Feb 9, 2022
65a0ede
conf-parser: add specific parser for PID values
poettering Feb 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/core/load-fragment.c
View file
Open in desktop
Expand Up @@ -6173,6 +6173,7 @@ void unit_dump_config_items(FILE *f) {
{ config_parse_si_uint64, "SIZE" },
{ config_parse_bool, "BOOLEAN" },
{ config_parse_string, "STRING" },
{ config_parse_safe_string, "STRING" },
{ config_parse_path, "PATH" },
{ config_parse_unit_path_printf, "PATH" },
{ config_parse_colon_separated_paths, "PATH" },
Expand Down
2 changes: 1 addition & 1 deletion src/journal-remote/journal-upload.c
View file
Open in desktop
Expand Up @@ -569,7 +569,7 @@ static int config_parse_path_or_ignore(

static int parse_config(void) {
const ConfigTableItem items[] = {
{ "Upload", "URL", config_parse_string, 0, &arg_url },
{ "Upload", "URL", config_parse_safe_string, 0, &arg_url },
{ "Upload", "ServerKeyFile", config_parse_path_or_ignore, 0, &arg_key },
{ "Upload", "ServerCertificateFile", config_parse_path_or_ignore, 0, &arg_cert },
{ "Upload", "TrustedCertificateFile", config_parse_path_or_ignore, 0, &arg_trust },
Expand Down
8 changes: 4 additions & 4 deletions src/network/netdev/netdev-gperf.gperf
View file
Open in desktop
Expand Up @@ -178,14 +178,14 @@ Tun.OneQueue, config_parse_warn_compat,
Tun.MultiQueue, config_parse_bool, 0, offsetof(TunTap, multi_queue)
Tun.PacketInfo, config_parse_bool, 0, offsetof(TunTap, packet_info)
Tun.VNetHeader, config_parse_bool, 0, offsetof(TunTap, vnet_hdr)
Tun.User, config_parse_string, 0, offsetof(TunTap, user_name)
Tun.Group, config_parse_string, 0, offsetof(TunTap, group_name)
Tun.User, config_parse_safe_string, 0, offsetof(TunTap, user_name)
Tun.Group, config_parse_safe_string, 0, offsetof(TunTap, group_name)
Tap.OneQueue, config_parse_warn_compat, DISABLED_LEGACY, 0
Tap.MultiQueue, config_parse_bool, 0, offsetof(TunTap, multi_queue)
Tap.PacketInfo, config_parse_bool, 0, offsetof(TunTap, packet_info)
Tap.VNetHeader, config_parse_bool, 0, offsetof(TunTap, vnet_hdr)
Tap.User, config_parse_string, 0, offsetof(TunTap, user_name)
Tap.Group, config_parse_string, 0, offsetof(TunTap, group_name)
Tap.User, config_parse_safe_string, 0, offsetof(TunTap, user_name)
Tap.Group, config_parse_safe_string, 0, offsetof(TunTap, group_name)
Bond.Mode, config_parse_bond_mode, 0, offsetof(Bond, mode)
Bond.TransmitHashPolicy, config_parse_bond_xmit_hash_policy, 0, offsetof(Bond, xmit_hash_policy)
Bond.LACPTransmitRate, config_parse_bond_lacp_rate, 0, offsetof(Bond, lacp_rate)
Expand Down
4 changes: 2 additions & 2 deletions src/network/networkd-network-gperf.gperf
View file
Open in desktop
Expand Up @@ -220,7 +220,7 @@ DHCPv4.SendHostname, config_parse_bool,
DHCPv4.Hostname, config_parse_hostname, 0, offsetof(Network, dhcp_hostname)
DHCPv4.Label, config_parse_dhcp_label, 0, offsetof(Network, dhcp_label)
DHCPv4.RequestBroadcast, config_parse_tristate, 0, offsetof(Network, dhcp_broadcast)
DHCPv4.VendorClassIdentifier, config_parse_string, 0, offsetof(Network, dhcp_vendor_class_identifier)
DHCPv4.VendorClassIdentifier, config_parse_safe_string, 0, offsetof(Network, dhcp_vendor_class_identifier)
DHCPv4.MUDURL, config_parse_mud_url, 0, offsetof(Network, dhcp_mudurl)
DHCPv4.MaxAttempts, config_parse_dhcp_max_attempts, 0, 0
DHCPv4.UserClass, config_parse_dhcp_user_or_vendor_class, AF_INET, offsetof(Network, dhcp_user_class)
Expand Down Expand Up @@ -539,7 +539,7 @@ DHCP.SendHostname, config_parse_bool,
DHCP.Hostname, config_parse_hostname, 0, offsetof(Network, dhcp_hostname)
DHCP.RequestBroadcast, config_parse_tristate, 0, offsetof(Network, dhcp_broadcast)
DHCP.CriticalConnection, config_parse_tristate, 0, offsetof(Network, dhcp_critical)
DHCP.VendorClassIdentifier, config_parse_string, 0, offsetof(Network, dhcp_vendor_class_identifier)
DHCP.VendorClassIdentifier, config_parse_safe_string, 0, offsetof(Network, dhcp_vendor_class_identifier)
DHCP.UserClass, config_parse_dhcp_user_or_vendor_class, AF_INET, offsetof(Network, dhcp_user_class)
DHCP.IAID, config_parse_iaid, AF_INET, 0
DHCP.DUIDType, config_parse_network_duid_type, 0, 0
Expand Down
2 changes: 1 addition & 1 deletion src/nspawn/nspawn-gperf.gperf
View file
Open in desktop
Expand Up @@ -24,7 +24,7 @@ Exec.Ephemeral, config_parse_tristate, 0, of
Exec.ProcessTwo, config_parse_pid2, 0, 0
Exec.Parameters, config_parse_strv, 0, offsetof(Settings, parameters)
Exec.Environment, config_parse_strv, 0, offsetof(Settings, environment)
Exec.User, config_parse_string, 0, offsetof(Settings, user)
Exec.User, config_parse_safe_string, 0, offsetof(Settings, user)
Exec.Capability, config_parse_capability, 0, offsetof(Settings, capability)
Exec.AmbientCapability, config_parse_capability, 0, offsetof(Settings, ambient_capability)
Exec.DropCapability, config_parse_capability, 0, offsetof(Settings, drop_capability)
Expand Down
101 changes: 69 additions & 32 deletions src/shared/conf-parser.c
View file
Open in desktop
Expand Up @@ -40,18 +40,18 @@ int config_item_table_lookup(
const void *table,
const char *section,
const char *lvalue,
ConfigParserCallback *func,
int *ltype,
void **data,
ConfigParserCallback *ret_func,
int *ret_ltype,
void **ret_data,
void *userdata) {

const ConfigTableItem *t;

assert(table);
assert(lvalue);
assert(func);
assert(ltype);
assert(data);
assert(ret_func);
assert(ret_ltype);
assert(ret_data);

for (t = table; t->lvalue; t++) {

Expand All @@ -61,32 +61,35 @@ int config_item_table_lookup(
if (!streq_ptr(section, t->section))
continue;

*func = t->parse;
*ltype = t->ltype;
*data = t->data;
*ret_func = t->parse;
*ret_ltype = t->ltype;
*ret_data = t->data;
return 1;
}

*ret_func = NULL;
*ret_ltype = 0;
*ret_data = NULL;
return 0;
}

int config_item_perf_lookup(
const void *table,
const char *section,
const char *lvalue,
ConfigParserCallback *func,
int *ltype,
void **data,
ConfigParserCallback *ret_func,
int *ret_ltype,
void **ret_data,
void *userdata) {

ConfigPerfItemLookup lookup = (ConfigPerfItemLookup) table;
const ConfigPerfItem *p;

assert(table);
assert(lvalue);
assert(func);
assert(ltype);
assert(data);
assert(ret_func);
assert(ret_ltype);
assert(ret_data);

if (section) {
const char *key;
Expand All @@ -95,12 +98,16 @@ int config_item_perf_lookup(
p = lookup(key, strlen(key));
} else
p = lookup(lvalue, strlen(lvalue));
if (!p)
if (!p) {
*ret_func = NULL;
*ret_ltype = 0;
*ret_data = NULL;
return 0;
}

*func = p->parse;
*ltype = p->ltype;
*data = (uint8_t*) userdata + p->offset;
*ret_func = p->parse;
*ret_ltype = p->ltype;
*ret_data = (uint8_t*) userdata + p->offset;
return 1;
}

Expand Down Expand Up @@ -133,11 +140,11 @@ static int next_assignment(
if (r < 0)
return r;
if (r > 0) {
if (func)
return func(unit, filename, line, section, section_line,
lvalue, ltype, rvalue, data, userdata);
if (!func)
return 0;

return 0;
return func(unit, filename, line, section, section_line,
lvalue, ltype, rvalue, data, userdata);
}

/* Warn about unknown non-extension fields. */
Expand All @@ -160,7 +167,7 @@ static int parse_line(
char **section,
unsigned *section_line,
bool *section_ignored,
char *l,
char *l, /* is modified */
void *userdata) {

char *e;
Expand All @@ -171,18 +178,18 @@ static int parse_line(
assert(l);

l = strstrip(l);
if (!*l)
if (isempty(l))
return 0;

if (*l == '\n')
if (l[0] == '\n')
return 0;

if (!utf8_is_valid(l))
return log_syntax_invalid_utf8(unit, LOG_WARNING, filename, line, l);

if (*l == '[') {
if (l[0] == '[') {
_cleanup_free_ char *n = NULL;
size_t k;
char *n;

k = strlen(l);
assert(k > 0);
Expand All @@ -194,23 +201,25 @@ static int parse_line(
if (!n)
return log_oom();

if (!string_is_safe(n))
return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EBADMSG), "Bad characters in section header '%s'", l);

if (sections && !nulstr_contains(sections, n)) {
bool ignore = flags & CONFIG_PARSE_RELAXED;
bool ignore;
const char *t;

ignore = ignore || startswith(n, "X-");
ignore = (flags & CONFIG_PARSE_RELAXED) || startswith(n, "X-");

if (!ignore)
NULSTR_FOREACH(t, sections)
if (streq_ptr(n, startswith(t, "-"))) {
if (streq_ptr(n, startswith(t, "-"))) { /* Ignore sections prefixed with "-" in valid section list */
ignore = true;
break;
}

if (!ignore)
log_syntax(unit, LOG_WARNING, filename, line, 0, "Unknown section '%s'. Ignoring.", n);

free(n);
*section = mfree(*section);
*section_line = 0;
*section_ignored = true;
Expand Down Expand Up @@ -633,6 +642,7 @@ DEFINE_PARSER(nsec, nsec_t, parse_nsec);
DEFINE_PARSER(sec, usec_t, parse_sec);
DEFINE_PARSER(sec_def_infinity, usec_t, parse_sec_def_infinity);
DEFINE_PARSER(mode, mode_t, parse_mode);
DEFINE_PARSER(pid, pid_t, parse_pid);

int config_parse_iec_size(
const char* unit,
Expand Down Expand Up @@ -873,6 +883,33 @@ int config_parse_string(
return free_and_strdup_warn(s, empty_to_null(rvalue));
}

int config_parse_safe_string(
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) {

char **s = data;

assert(filename);
assert(lvalue);
assert(rvalue);
assert(data);

if (!string_is_safe(rvalue)) {
log_syntax(unit, LOG_WARNING, filename, line, 0, "Specified string contains unsafe characters, ignoring: %s", rvalue);
return 0;
}

return free_and_strdup_warn(s, empty_to_null(rvalue));
}

int config_parse_path(
const char *unit,
const char *filename,
Expand Down
12 changes: 7 additions & 5 deletions src/shared/conf-parser.h
View file
Open in desktop
Expand Up @@ -69,18 +69,18 @@ typedef int (*ConfigItemLookup)(
const void *table,
const char *section,
const char *lvalue,
ConfigParserCallback *func,
int *ltype,
void **data,
ConfigParserCallback *ret_func,
int *ret_ltype,
void **ret_data,
void *userdata);

/* Linear table search implementation of ConfigItemLookup, based on
* ConfigTableItem arrays */
int config_item_table_lookup(const void *table, const char *section, const char *lvalue, ConfigParserCallback *func, int *ltype, void **data, void *userdata);
int config_item_table_lookup(const void *table, const char *section, const char *lvalue, ConfigParserCallback *ret_func, int *ret_ltype, void **ret_data, void *userdata);

/* gperf implementation of ConfigItemLookup, based on gperf
* ConfigPerfItem tables */
int config_item_perf_lookup(const void *table, const char *section, const char *lvalue, ConfigParserCallback *func, int *ltype, void **data, void *userdata);
int config_item_perf_lookup(const void *table, const char *section, const char *lvalue, ConfigParserCallback *ret_func, int *ret_ltype, void **ret_data, void *userdata);

int config_parse(
const char *unit,
Expand Down Expand Up @@ -168,6 +168,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_bool);
CONFIG_PARSER_PROTOTYPE(config_parse_id128);
CONFIG_PARSER_PROTOTYPE(config_parse_tristate);
CONFIG_PARSER_PROTOTYPE(config_parse_string);
CONFIG_PARSER_PROTOTYPE(config_parse_safe_string);
CONFIG_PARSER_PROTOTYPE(config_parse_path);
CONFIG_PARSER_PROTOTYPE(config_parse_strv);
CONFIG_PARSER_PROTOTYPE(config_parse_sec);
Expand All @@ -194,6 +195,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_ether_addrs);
CONFIG_PARSER_PROTOTYPE(config_parse_in_addr_non_null);
CONFIG_PARSER_PROTOTYPE(config_parse_percent);
CONFIG_PARSER_PROTOTYPE(config_parse_permyriad);
CONFIG_PARSER_PROTOTYPE(config_parse_pid);

typedef enum Disabled {
DISABLED_CONFIGURATION,
Expand Down
22 changes: 11 additions & 11 deletions src/tty-ask-password-agent/tty-ask-password-agent.c
View file
Open in desktop
Expand Up @@ -174,16 +174,16 @@ static int process_one_password_file(const char *filename) {
_cleanup_free_ char *socket_name = NULL, *message = NULL;
bool accept_cached = false, echo = false, silent = false;
uint64_t not_after = 0;
unsigned pid = 0;
pid_t pid = 0;

const ConfigTableItem items[] = {
{ "Ask", "Socket", config_parse_string, 0, &socket_name },
{ "Ask", "NotAfter", config_parse_uint64, 0, &not_after },
{ "Ask", "Message", config_parse_string, 0, &message },
{ "Ask", "PID", config_parse_unsigned, 0, &pid },
{ "Ask", "AcceptCached", config_parse_bool, 0, &accept_cached },
{ "Ask", "Echo", config_parse_bool, 0, &echo },
{ "Ask", "Silent", config_parse_bool, 0, &silent },
{ "Ask", "Socket", config_parse_safe_string, 0, &socket_name },
{ "Ask", "NotAfter", config_parse_uint64, 0, &not_after },
{ "Ask", "Message", config_parse_string, 0, &message },
{ "Ask", "PID", config_parse_pid, 0, &pid },
{ "Ask", "AcceptCached", config_parse_bool, 0, &accept_cached },
{ "Ask", "Echo", config_parse_bool, 0, &echo },
{ "Ask", "Silent", config_parse_bool, 0, &silent },
{}
};

Expand Down Expand Up @@ -212,14 +212,14 @@ static int process_one_password_file(const char *filename) {

switch (arg_action) {
case ACTION_LIST:
printf("'%s' (PID %u)\n", strna(message), pid);
printf("'%s' (PID " PID_FMT ")\n", strna(message), pid);
return 0;

case ACTION_WALL: {
_cleanup_free_ char *wall = NULL;

if (asprintf(&wall,
"Password entry required for \'%s\' (PID %u).\r\n"
"Password entry required for \'%s\' (PID " PID_FMT ").\r\n"
"Please enter password with the systemd-tty-ask-password-agent tool.",
strna(message),
pid) < 0)
Expand All @@ -235,7 +235,7 @@ static int process_one_password_file(const char *filename) {

if (access(socket_name, W_OK) < 0) {
if (arg_action == ACTION_QUERY)
log_info("Not querying '%s' (PID %u), lacking privileges.", strna(message), pid);
log_info("Not querying '%s' (PID " PID_FMT "), lacking privileges.", strna(message), pid);

return 0;
}
Expand Down