Skip to content

Commit

Permalink
Merge pull request #22458 from poettering/parse-safe-string
Browse files Browse the repository at this point in the history 
some safety tweaks to conf-parser.[ch]
  • Loading branch information
@bluca
bluca committed Feb 9, 2022
2 parents 0628d48 + 65a0ede commit 0b0ad49
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 63 deletions.
1 change: 1 addition & 0 deletions src/core/load-fragment.c
View file
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
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
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
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
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
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
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
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

0 comments on commit 0b0ad49

Please sign in to comment.