Merge pull request #27169 from yuwata/udev-rule-refuse-unsafe-path

sd-device,udev: refuse unsafe path in SYMLINK= and TAG=

1 files changed, 33 insertions, 30 deletions

diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 19bbf295c5..e3d2adbafd 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -35,7 +35,7 @@
 #include "user-util.h"
 #include "virt.h"
 
-#define RULES_DIRS (const char* const*) CONF_PATHS_STRV("udev/rules.d")
+#define RULES_DIRS ((const char* const*) CONF_PATHS_STRV("udev/rules.d"))
 
 typedef enum {
         OP_MATCH,        /* == */
@@ -2487,16 +2487,14 @@ static int udev_rule_apply_token_to_event(
                 if (token->op == OP_ASSIGN)
                         device_cleanup_tags(dev);
 
-                if (buf[strspn(buf, ALPHANUMERICAL "-_")] != '\0') {
-                        log_event_error(dev, token, "Invalid tag name '%s', ignoring", buf);
-                        break;
-                }
                 if (token->op == OP_REMOVE)
                         device_remove_tag(dev, buf);
                 else {
                         r = device_add_tag(dev, buf, true);
+                        if (r == -ENOMEM)
+                                return log_oom();
                         if (r < 0)
-                                return log_event_error_errno(dev, token, r, "Failed to add tag '%s': %m", buf);
+                                log_event_warning_errno(dev, token, r, "Failed to add tag '%s', ignoring: %m", buf);
                 }
                 break;
         }
@@ -2541,7 +2539,7 @@ static int udev_rule_apply_token_to_event(
                 break;
         }
         case TK_A_DEVLINK: {
-                char buf[UDEV_PATH_SIZE], *p;
+                char buf[UDEV_PATH_SIZE];
                 bool truncated;
                 size_t count;
 
@@ -2554,17 +2552,18 @@ static int udev_rule_apply_token_to_event(
                 if (IN_SET(token->op, OP_ASSIGN, OP_ASSIGN_FINAL))
                         device_cleanup_devlinks(dev);
 
-                /* allow multiple symlinks separated by spaces */
-                (void) udev_event_apply_format(event, token->value, buf, sizeof(buf), event->esc != ESCAPE_NONE, &truncated);
+                (void) udev_event_apply_format(event, token->value, buf, sizeof(buf),
+                                               /* replace_whitespace = */ event->esc != ESCAPE_NONE, &truncated);
                 if (truncated) {
                         log_event_truncated(dev, token, "symbolic link path", token->value, "SYMLINK", /* is_match = */ false);
                         break;
                 }
 
+                /* By default or string_escape=none, allow multiple symlinks separated by spaces. */
                 if (event->esc == ESCAPE_UNSET)
-                        count = udev_replace_chars(buf, "/ ");
+                        count = udev_replace_chars(buf, /* allow = */ "/ ");
                 else if (event->esc == ESCAPE_REPLACE)
-                        count = udev_replace_chars(buf, "/");
+                        count = udev_replace_chars(buf, /* allow = */ "/");
                 else
                         count = 0;
                 if (count > 0)
@@ -2572,32 +2571,36 @@ static int udev_rule_apply_token_to_event(
                                         "Replaced %zu character(s) from result of SYMLINK=\"%s\"",
                                         count, token->value);
 
-                p = skip_leading_chars(buf, NULL);
-                while (!isempty(p)) {
-                        char filename[UDEV_PATH_SIZE], *next;
+                for (const char *p = buf;;) {
+                        _cleanup_free_ char *path = NULL;
 
-                        next = strchr(p, ' ');
-                        if (next) {
-                                *next++ = '\0';
-                                next = skip_leading_chars(next, NULL);
+                        r = extract_first_word(&p, &path, NULL, EXTRACT_RETAIN_ESCAPE);
+                        if (r == -ENOMEM)
+                                return log_oom();
+                        if (r < 0) {
+                                log_warning_errno(r, "Failed to extract first path in SYMLINK=, ignoring: %m");
+                                break;
                         }
-
-                        strscpyl_full(filename, sizeof(filename), &truncated, "/dev/", p, NULL);
-                        if (truncated)
-                                continue;
+                        if (r == 0)
+                                break;
 
                         if (token->op == OP_REMOVE) {
-                                device_remove_devlink(dev, filename);
-                                log_event_debug(dev, token, "Dropped SYMLINK '%s'", p);
+                                r = device_remove_devlink(dev, path);
+                                if (r == -ENOMEM)
+                                        return log_oom();
+                                if (r < 0)
+                                        log_event_warning_errno(dev, token, r, "Failed to remove devlink '%s', ignoring: %m", path);
+                                else if (r > 0)
+                                        log_event_debug(dev, token, "Dropped SYMLINK '%s'", path);
                         } else {
-                                r = device_add_devlink(dev, filename);
+                                r = device_add_devlink(dev, path);
+                                if (r == -ENOMEM)
+                                        return log_oom();
                                 if (r < 0)
-                                        return log_event_error_errno(dev, token, r, "Failed to add devlink '%s': %m", filename);
-
-                                log_event_debug(dev, token, "Added SYMLINK '%s'", p);
+                                        log_event_warning_errno(dev, token, r, "Failed to add devlink '%s', ignoring: %m", path);
+                                else if (r > 0)
+                                        log_event_debug(dev, token, "Added SYMLINK '%s'", path);
                         }
-
-                        p = next;
                 }
                 break;
         }