udev: check formatting of attribute or value earlier

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 32788c05e06cf8f6ed13b640bf1c5a2d14b6f4dd..680320b8675271574dcade3c355568494489633c 100644 (file)
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -149,7 +149,7 @@ static char format_type_to_char(FormatSubstitutionType t) {
         return '\0';
 }
 
-static int get_subst_type(const char **str, FormatSubstitutionType *ret_type, char ret_attr[static UTIL_PATH_SIZE]) {
+static int get_subst_type(const char **str, bool strict, FormatSubstitutionType *ret_type, char ret_attr[static UTIL_PATH_SIZE]) {
         const char *p = *str, *q = NULL;
         size_t i;
 
@@ -178,9 +178,11 @@ static int get_subst_type(const char **str, FormatSubstitutionType *ret_type, ch
                                 q = p + 1;
                                 break;
                         }
-        }
-        if (!q)
+        } else
                 return 0;
+        if (!q)
+                /* When 'strict' flag is set, then '$' and '%' must be escaped. */
+                return strict ? -EINVAL : 0;
 
         if (q[0] == '{') {
                 const char *start, *end;
@@ -450,7 +452,7 @@ ssize_t udev_event_apply_format(UdevEvent *event,
                 char attr[UTIL_PATH_SIZE];
                 ssize_t subst_len;
 
-                r = get_subst_type(&s, &type, attr);
+                r = get_subst_type(&s, false, &type, attr);
                 if (r < 0)
                         return log_device_warning_errno(event->dev, r, "Invalid format string, ignoring: %s", src);
                 if (r == 0) {
@@ -482,6 +484,35 @@ ssize_t udev_event_apply_format(UdevEvent *event,
         return size;
 }
 
+int udev_check_format(const char *s) {
+        FormatSubstitutionType type;
+        char attr[UTIL_PATH_SIZE];
+        int r;
+
+        while (s[0] != '\0') {
+                r = get_subst_type(&s, true, &type, attr);
+                if (r < 0)
+                        return r;
+                if (r == 0) {
+                        s++;
+                        continue;
+                }
+
+                if (IN_SET(type, FORMAT_SUBST_ATTR, FORMAT_SUBST_ENV) && isempty(attr))
+                        return -EINVAL;
+
+                if (type == FORMAT_SUBST_RESULT && !isempty(attr)) {
+                        unsigned i;
+
+                        r = safe_atou_optional_plus(attr, &i);
+                        if (r < 0)
+                                return r;
+                }
+        }
+
+        return 0;
+}
+
 static int on_spawn_io(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         Spawn *spawn = userdata;
         char buf[4096], *p;

diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h
index 9f5e104bd4283b654f124941271638b6e54a7719..eca4ec8a9d021130a7946eaa1380edc03a210bfe 100644 (file)
--- a/src/udev/udev-event.h
+++ b/src/udev/udev-event.h
@@ -51,6 +51,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(UdevEvent*, udev_event_free);
 ssize_t udev_event_apply_format(UdevEvent *event,
                                 const char *src, char *dest, size_t size,
                                 bool replace_whitespace);
+int udev_check_format(const char *s);
 int udev_event_spawn(UdevEvent *event,
                      usec_t timeout_usec,
                      bool accept_failure,

diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 149d62afb32c2673747ddda1db12be90e8ad086a..6811a5ce4b775dcade9bc159bc4e1c8f88a965b5 100644 (file)
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -345,7 +345,13 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
 
-                r = rule_line_add_token(rule_line, is_match ? TK_M_DEVLINK : TK_A_DEVLINK, op, value, NULL);
+                if (!is_match) {
+                        if (udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
+
+                        r = rule_line_add_token(rule_line, TK_A_DEVLINK, op, value, NULL);
+                } else
+                        r = rule_line_add_token(rule_line, TK_M_DEVLINK, op, value, NULL);
         } else if (streq(key, "NAME")) {
                 if (attr)
                         return log_token_invalid_attr(rules, key);
@@ -363,6 +369,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         if (isempty(value))
                                 return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
                                                              "Ignoring NAME=\"\", as udev will not delete any device nodes.");
+                        if (udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
+
                         r = rule_line_add_token(rule_line, TK_A_NAME, op, value, NULL);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_NAME, op, value, NULL);
@@ -383,6 +392,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                                 return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
                                                              "Invalid ENV attribute. '%s' cannot be set.", attr);
 
+                        if (udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
                         r = rule_line_add_token(rule_line, TK_A_ENV, op, value, attr);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_ENV, op, value, attr);
@@ -394,7 +405,12 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         op = OP_ASSIGN;
                 }
 
-                r = rule_line_add_token(rule_line, is_match ? TK_M_TAG : TK_A_TAG, op, value, NULL);
+                if (!is_match) {
+                        if (isempty(value) || udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
+                        r = rule_line_add_token(rule_line, TK_A_TAG, op, value, NULL);
+                } else
+                        r = rule_line_add_token(rule_line, TK_M_TAG, op, value, NULL);
         } else if (streq(key, "SUBSYSTEM")) {
                 if (attr)
                         return log_token_invalid_attr(rules, key);
@@ -414,7 +430,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                 r = rule_line_add_token(rule_line, TK_M_DRIVER, op, value, NULL);
         } else if (streq(key, "ATTR")) {
                 if (isempty(attr))
-                        return log_token_invalid_op(rules, key);
+                        return log_token_invalid_attr(rules, key);
+                if (udev_check_format(attr) < 0)
+                        log_token_invalid_attr_format(rules, key, attr);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (IN_SET(op, OP_ADD, OP_ASSIGN_FINAL)) {
@@ -422,10 +440,18 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         op = OP_ASSIGN;
                 }
 
-                r = rule_line_add_token(rule_line, is_match ? TK_M_ATTR : TK_A_ATTR, op, value, attr);
+                if (!is_match) {
+                        if (udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
+
+                        r = rule_line_add_token(rule_line, TK_A_ATTR, op, value, attr);
+                } else
+                        r = rule_line_add_token(rule_line, TK_M_ATTR, op, value, attr);
         } else if (streq(key, "SYSCTL")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
+                if (udev_check_format(attr) < 0)
+                        log_token_invalid_attr_format(rules, key, attr);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (IN_SET(op, OP_ADD, OP_ASSIGN_FINAL)) {
@@ -433,7 +459,13 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         op = OP_ASSIGN;
                 }
 
-                r = rule_line_add_token(rule_line, is_match ? TK_M_SYSCTL : TK_A_SYSCTL, op, value, attr);
+                if (!is_match) {
+                        if (udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
+
+                        r = rule_line_add_token(rule_line, TK_A_SYSCTL, op, value, attr);
+                } else
+                        r = rule_line_add_token(rule_line, TK_M_SYSCTL, op, value, attr);
         } else if (streq(key, "KERNELS")) {
                 if (attr)
                         return log_token_invalid_attr(rules, key);
@@ -458,6 +490,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "ATTRS")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
+                if (udev_check_format(attr) < 0)
+                        log_token_invalid_attr_format(rules, key, attr);
                 if (!is_match)
                         return log_token_invalid_op(rules, key);
 
@@ -482,6 +516,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         if (r < 0)
                                 return log_token_error_errno(rules, r, "Failed to parse mode '%s': %m", attr);
                 }
+                if (isempty(value) || udev_check_format(value) < 0)
+                        log_token_invalid_value(rules, key, value);
                 if (!is_match)
                         return log_token_invalid_op(rules, key);
 
@@ -489,6 +525,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "PROGRAM")) {
                 if (attr)
                         return log_token_invalid_attr(rules, key);
+                if (isempty(value) || udev_check_format(value) < 0)
+                        log_token_invalid_value(rules, key, value);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (!is_match) {
@@ -503,6 +541,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "IMPORT")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
+                if (isempty(value) || udev_check_format(value) < 0)
+                        log_token_invalid_value(rules, key, value);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (!is_match) {
@@ -603,9 +643,11 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                                 return log_token_error_errno(rules, r, "Failed to resolve user name '%s': %m", value);
 
                         r = rule_line_add_token(rule_line, TK_A_OWNER_ID, op, NULL, UID_TO_PTR(uid));
-                } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER)
+                } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) {
+                        if (isempty(value) || udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
                         r = rule_line_add_token(rule_line, TK_A_OWNER, op, value, NULL);
-                else {
+                } else {
                         log_token_debug(rules, "Resolving user name is disabled, ignoring %s=%s", key, value);
                         return 0;
                 }
@@ -630,9 +672,11 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                                 return log_token_error_errno(rules, r, "Failed to resolve group name '%s': %m", value);
 
                         r = rule_line_add_token(rule_line, TK_A_GROUP_ID, op, NULL, GID_TO_PTR(gid));
-                } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER)
+                } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) {
+                        if (isempty(value) || udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
                         r = rule_line_add_token(rule_line, TK_A_GROUP, op, value, NULL);
-                else {
+                } else {
                         log_token_debug(rules, "Resolving group name is disabled, ignoring %s=%s", key, value);
                         return 0;
                 }
@@ -650,11 +694,16 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
 
                 if (parse_mode(value, &mode) >= 0)
                         r = rule_line_add_token(rule_line, TK_A_MODE_ID, op, NULL, MODE_TO_PTR(mode));
-                else
+                else {
+                        if (isempty(value) || udev_check_format(value) < 0)
+                                log_token_invalid_value(rules, key, value);
                         r = rule_line_add_token(rule_line, TK_A_MODE, op, value, NULL);
+                }
         } else if (streq(key, "SECLABEL")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
+                if (isempty(value) || udev_check_format(value) < 0)
+                        log_token_invalid_value(rules, key, value);
                 if (is_match || op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (op == OP_ASSIGN_FINAL) {
@@ -666,6 +715,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "RUN")) {
                 if (is_match || op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
+                if (isempty(value) || udev_check_format(value) < 0)
+                        log_token_invalid_value(rules, key, value);
                 if (!attr || streq(attr, "program"))
                         r = rule_line_add_token(rule_line, TK_A_RUN_PROGRAM, op, value, NULL);
                 else if (streq(attr, "builtin")) {

diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h
index a5cb5a0d3227d4b8d2091bff1d65d3c349add0d7..f5f5bdb060b54088d5d60f0cc5e4b6ee3d9925ee 100644 (file)
--- a/src/udev/udev-rules.h
+++ b/src/udev/udev-rules.h
@@ -215,3 +215,12 @@ int udev_rules_apply_static_dev_perms(UdevRules *rules);
 
 #define log_token_invalid_op(rules, key)   _log_token_invalid(rules, key, "operator")
 #define log_token_invalid_attr(rules, key) _log_token_invalid(rules, key, "attribute")
+
+#define log_token_invalid_attr_format(rules, key, attr)                 \
+        log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),           \
+                              "Invalid attribute \"%s\" for %s, ignoring, but please fix it.", \
+                              attr, key)
+#define log_token_invalid_value(rules, key, value)                      \
+        log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),           \
+                              "Invalid value \"%s\" for %s, ignoring, but please fix it.", \
+                              value, key)