Skip to content

Commit d7aee41

Browse files
yuwatayuwata
committed
udev: check formatting of attribute or value earlier
1 parent 1448820 commit d7aee41

File tree

4 files changed

+106
-14
lines changed

4 files changed

+106
-14
lines changed

‎src/udev/udev-event.c‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ static char format_type_to_char(FormatSubstitutionType t) {
149149
return '\0';
150150
}
151151

152-
static int get_subst_type(const char **str, FormatSubstitutionType *ret_type, char ret_attr[static UTIL_PATH_SIZE]) {
152+
static int get_subst_type(const char **str, bool strict, FormatSubstitutionType *ret_type, char ret_attr[static UTIL_PATH_SIZE]) {
153153
const char *p = *str, *q = NULL;
154154
size_t i;
155155

@@ -178,9 +178,11 @@ static int get_subst_type(const char **str, FormatSubstitutionType *ret_type, ch
178178
q = p + 1;
179179
break;
180180
}
181-
}
182-
if (!q)
181+
} else
183182
return 0;
183+
if (!q)
184+
/* When 'strict' flag is set, then '$' and '%' must be escaped. */
185+
return strict ? -EINVAL : 0;
184186

185187
if (q[0] == '{') {
186188
const char *start, *end;
@@ -450,7 +452,7 @@ ssize_t udev_event_apply_format(UdevEvent *event,
450452
char attr[UTIL_PATH_SIZE];
451453
ssize_t subst_len;
452454

453-
r = get_subst_type(&s, &type, attr);
455+
r = get_subst_type(&s, false, &type, attr);
454456
if (r < 0)
455457
return log_device_warning_errno(event->dev, r, "Invalid format string, ignoring: %s", src);
456458
if (r == 0) {
@@ -482,6 +484,35 @@ ssize_t udev_event_apply_format(UdevEvent *event,
482484
return size;
483485
}
484486

487+
int udev_check_format(const char *s) {
488+
FormatSubstitutionType type;
489+
char attr[UTIL_PATH_SIZE];
490+
int r;
491+
492+
while (s[0] != '\0') {
493+
r = get_subst_type(&s, true, &type, attr);
494+
if (r < 0)
495+
return r;
496+
if (r == 0) {
497+
s++;
498+
continue;
499+
}
500+
501+
if (IN_SET(type, FORMAT_SUBST_ATTR, FORMAT_SUBST_ENV) && isempty(attr))
502+
return -EINVAL;
503+
504+
if (type == FORMAT_SUBST_RESULT && !isempty(attr)) {
505+
unsigned i;
506+
507+
r = safe_atou_optional_plus(attr, &i);
508+
if (r < 0)
509+
return r;
510+
}
511+
}
512+
513+
return 0;
514+
}
515+
485516
static int on_spawn_io(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
486517
Spawn *spawn = userdata;
487518
char buf[4096], *p;

‎src/udev/udev-event.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(UdevEvent*, udev_event_free);
5151
ssize_t udev_event_apply_format(UdevEvent *event,
5252
const char *src, char *dest, size_t size,
5353
bool replace_whitespace);
54+
int udev_check_format(const char *s);
5455
int udev_event_spawn(UdevEvent *event,
5556
usec_t timeout_usec,
5657
bool accept_failure,

‎src/udev/udev-rules.c‎

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,13 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
345345
if (op == OP_REMOVE)
346346
return log_token_invalid_op(rules, key);
347347

348-
r = rule_line_add_token(rule_line, is_match ? TK_M_DEVLINK : TK_A_DEVLINK, op, value, NULL);
348+
if (!is_match) {
349+
if (udev_check_format(value) < 0)
350+
log_token_invalid_value(rules, key, value);
351+
352+
r = rule_line_add_token(rule_line, TK_A_DEVLINK, op, value, NULL);
353+
} else
354+
r = rule_line_add_token(rule_line, TK_M_DEVLINK, op, value, NULL);
349355
} else if (streq(key, "NAME")) {
350356
if (attr)
351357
return log_token_invalid_attr(rules, key);
@@ -363,6 +369,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
363369
if (isempty(value))
364370
return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
365371
"Ignoring NAME=\"\", as udev will not delete any device nodes.");
372+
if (udev_check_format(value) < 0)
373+
log_token_invalid_value(rules, key, value);
374+
366375
r = rule_line_add_token(rule_line, TK_A_NAME, op, value, NULL);
367376
} else
368377
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
383392
return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
384393
"Invalid ENV attribute. '%s' cannot be set.", attr);
385394

395+
if (udev_check_format(value) < 0)
396+
log_token_invalid_value(rules, key, value);
386397
r = rule_line_add_token(rule_line, TK_A_ENV, op, value, attr);
387398
} else
388399
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
394405
op = OP_ASSIGN;
395406
}
396407

397-
r = rule_line_add_token(rule_line, is_match ? TK_M_TAG : TK_A_TAG, op, value, NULL);
408+
if (!is_match) {
409+
if (isempty(value) || udev_check_format(value) < 0)
410+
log_token_invalid_value(rules, key, value);
411+
r = rule_line_add_token(rule_line, TK_A_TAG, op, value, NULL);
412+
} else
413+
r = rule_line_add_token(rule_line, TK_M_TAG, op, value, NULL);
398414
} else if (streq(key, "SUBSYSTEM")) {
399415
if (attr)
400416
return log_token_invalid_attr(rules, key);
@@ -414,26 +430,42 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
414430
r = rule_line_add_token(rule_line, TK_M_DRIVER, op, value, NULL);
415431
} else if (streq(key, "ATTR")) {
416432
if (isempty(attr))
417-
return log_token_invalid_op(rules, key);
433+
return log_token_invalid_attr(rules, key);
434+
if (udev_check_format(attr) < 0)
435+
log_token_invalid_attr_format(rules, key, attr);
418436
if (op == OP_REMOVE)
419437
return log_token_invalid_op(rules, key);
420438
if (IN_SET(op, OP_ADD, OP_ASSIGN_FINAL)) {
421439
log_token_warning(rules, "%s key takes '==', '!=', or '=' operator, assuming '=', but please fix it.", key);
422440
op = OP_ASSIGN;
423441
}
424442

425-
r = rule_line_add_token(rule_line, is_match ? TK_M_ATTR : TK_A_ATTR, op, value, attr);
443+
if (!is_match) {
444+
if (udev_check_format(value) < 0)
445+
log_token_invalid_value(rules, key, value);
446+
447+
r = rule_line_add_token(rule_line, TK_A_ATTR, op, value, attr);
448+
} else
449+
r = rule_line_add_token(rule_line, TK_M_ATTR, op, value, attr);
426450
} else if (streq(key, "SYSCTL")) {
427451
if (isempty(attr))
428452
return log_token_invalid_attr(rules, key);
453+
if (udev_check_format(attr) < 0)
454+
log_token_invalid_attr_format(rules, key, attr);
429455
if (op == OP_REMOVE)
430456
return log_token_invalid_op(rules, key);
431457
if (IN_SET(op, OP_ADD, OP_ASSIGN_FINAL)) {
432458
log_token_warning(rules, "%s key takes '==', '!=', or '=' operator, assuming '=', but please fix it.", key);
433459
op = OP_ASSIGN;
434460
}
435461

436-
r = rule_line_add_token(rule_line, is_match ? TK_M_SYSCTL : TK_A_SYSCTL, op, value, attr);
462+
if (!is_match) {
463+
if (udev_check_format(value) < 0)
464+
log_token_invalid_value(rules, key, value);
465+
466+
r = rule_line_add_token(rule_line, TK_A_SYSCTL, op, value, attr);
467+
} else
468+
r = rule_line_add_token(rule_line, TK_M_SYSCTL, op, value, attr);
437469
} else if (streq(key, "KERNELS")) {
438470
if (attr)
439471
return log_token_invalid_attr(rules, key);
@@ -458,6 +490,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
458490
} else if (streq(key, "ATTRS")) {
459491
if (isempty(attr))
460492
return log_token_invalid_attr(rules, key);
493+
if (udev_check_format(attr) < 0)
494+
log_token_invalid_attr_format(rules, key, attr);
461495
if (!is_match)
462496
return log_token_invalid_op(rules, key);
463497

@@ -482,13 +516,17 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
482516
if (r < 0)
483517
return log_token_error_errno(rules, r, "Failed to parse mode '%s': %m", attr);
484518
}
519+
if (isempty(value) || udev_check_format(value) < 0)
520+
log_token_invalid_value(rules, key, value);
485521
if (!is_match)
486522
return log_token_invalid_op(rules, key);
487523

488524
r = rule_line_add_token(rule_line, TK_M_TEST, op, value, MODE_TO_PTR(mode));
489525
} else if (streq(key, "PROGRAM")) {
490526
if (attr)
491527
return log_token_invalid_attr(rules, key);
528+
if (isempty(value) || udev_check_format(value) < 0)
529+
log_token_invalid_value(rules, key, value);
492530
if (op == OP_REMOVE)
493531
return log_token_invalid_op(rules, key);
494532
if (!is_match) {
@@ -503,6 +541,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
503541
} else if (streq(key, "IMPORT")) {
504542
if (isempty(attr))
505543
return log_token_invalid_attr(rules, key);
544+
if (isempty(value) || udev_check_format(value) < 0)
545+
log_token_invalid_value(rules, key, value);
506546
if (op == OP_REMOVE)
507547
return log_token_invalid_op(rules, key);
508548
if (!is_match) {
@@ -603,9 +643,11 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
603643
return log_token_error_errno(rules, r, "Failed to resolve user name '%s': %m", value);
604644

605645
r = rule_line_add_token(rule_line, TK_A_OWNER_ID, op, NULL, UID_TO_PTR(uid));
606-
} else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER)
646+
} else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) {
647+
if (isempty(value) || udev_check_format(value) < 0)
648+
log_token_invalid_value(rules, key, value);
607649
r = rule_line_add_token(rule_line, TK_A_OWNER, op, value, NULL);
608-
else {
650+
} else {
609651
log_token_debug(rules, "Resolving user name is disabled, ignoring %s=%s", key, value);
610652
return 0;
611653
}
@@ -630,9 +672,11 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
630672
return log_token_error_errno(rules, r, "Failed to resolve group name '%s': %m", value);
631673

632674
r = rule_line_add_token(rule_line, TK_A_GROUP_ID, op, NULL, GID_TO_PTR(gid));
633-
} else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER)
675+
} else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) {
676+
if (isempty(value) || udev_check_format(value) < 0)
677+
log_token_invalid_value(rules, key, value);
634678
r = rule_line_add_token(rule_line, TK_A_GROUP, op, value, NULL);
635-
else {
679+
} else {
636680
log_token_debug(rules, "Resolving group name is disabled, ignoring %s=%s", key, value);
637681
return 0;
638682
}
@@ -650,11 +694,16 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
650694

651695
if (parse_mode(value, &mode) >= 0)
652696
r = rule_line_add_token(rule_line, TK_A_MODE_ID, op, NULL, MODE_TO_PTR(mode));
653-
else
697+
else {
698+
if (isempty(value) || udev_check_format(value) < 0)
699+
log_token_invalid_value(rules, key, value);
654700
r = rule_line_add_token(rule_line, TK_A_MODE, op, value, NULL);
701+
}
655702
} else if (streq(key, "SECLABEL")) {
656703
if (isempty(attr))
657704
return log_token_invalid_attr(rules, key);
705+
if (isempty(value) || udev_check_format(value) < 0)
706+
log_token_invalid_value(rules, key, value);
658707
if (is_match || op == OP_REMOVE)
659708
return log_token_invalid_op(rules, key);
660709
if (op == OP_ASSIGN_FINAL) {
@@ -666,6 +715,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
666715
} else if (streq(key, "RUN")) {
667716
if (is_match || op == OP_REMOVE)
668717
return log_token_invalid_op(rules, key);
718+
if (isempty(value) || udev_check_format(value) < 0)
719+
log_token_invalid_value(rules, key, value);
669720
if (!attr || streq(attr, "program"))
670721
r = rule_line_add_token(rule_line, TK_A_RUN_PROGRAM, op, value, NULL);
671722
else if (streq(attr, "builtin")) {

‎src/udev/udev-rules.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,12 @@ int udev_rules_apply_static_dev_perms(UdevRules *rules);
215215

216216
#define log_token_invalid_op(rules, key) _log_token_invalid(rules, key, "operator")
217217
#define log_token_invalid_attr(rules, key) _log_token_invalid(rules, key, "attribute")
218+
219+
#define log_token_invalid_attr_format(rules, key, attr) \
220+
log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \
221+
"Invalid attribute \"%s\" for %s, ignoring, but please fix it.", \
222+
attr, key)
223+
#define log_token_invalid_value(rules, key, value) \
224+
log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \
225+
"Invalid value \"%s\" for %s, ignoring, but please fix it.", \
226+
value, key)

0 commit comments

Comments
 (0)