[PATCH] remove untrusted chars read from sysfs-values or returned by PROGRAM
Better remove characters that are useless in a device node name. It may be a security risk to pass any character read from e.g. a sysfs attribute to a shell script we execute later. Prevent the modification of the libsysfs attribute value cache. Clear PROGRAM result if the execution encountered an error.
This commit is contained in:
kay.sievers@vrfy.org
Greg KH
parent
61b1b7069f
commit
18614ab25d
|
|
@ -1213,6 +1213,15 @@ BUS=="scsi", KERNEL=="sda1", ENV{ENV_KEY_TEST}=="go", NAME="wrong"
|
|||
BUS=="scsi", KERNEL=="sda1", ENV{ENV_KEY_TEST}=="yes", ENV{ACTION}=="add", ENV{DEVPATH}=="/block/sda/sdax1", NAME="no"
|
||||
BUS=="scsi", KERNEL=="sda1", ENV{ENV_KEY_TEST}=="test", ENV{ACTION}=="add", ENV{DEVPATH}=="/block/sda/sda1", NAME="true"
|
||||
BUS=="scsi", KERNEL=="sda1", ENV{ENV_KEY_TEST}=="bad", NAME="bad"
|
||||
EOF
|
||||
},
|
||||
{
|
||||
desc => "untrusted string sanitize",
|
||||
subsys => "block",
|
||||
devpath => "/block/sda/sda1",
|
||||
exp_name => "sane",
|
||||
rules => <<EOF
|
||||
BUS=="scsi", KERNEL=="sda1", PROGRAM=="/bin/echo -e name; (/sbin/badprogram)", RESULT="name_ _/sbin/badprogram_", NAME="sane"
|
||||
EOF
|
||||
},
|
||||
);
|
||||
|
|
|
|||
|
|
@ -182,19 +182,19 @@ static int parse_config_file(void)
|
|||
|
||||
if (strcasecmp(variable, "udev_root") == 0) {
|
||||
strlcpy(udev_root, value, sizeof(udev_root));
|
||||
no_trailing_slash(udev_root);
|
||||
remove_trailing_char(udev_root, '/');
|
||||
continue;
|
||||
}
|
||||
|
||||
if (strcasecmp(variable, "udev_db") == 0) {
|
||||
strlcpy(udev_db_path, value, sizeof(udev_db_path));
|
||||
no_trailing_slash(udev_db_path);
|
||||
remove_trailing_char(udev_db_path, '/');
|
||||
continue;
|
||||
}
|
||||
|
||||
if (strcasecmp(variable, "udev_rules") == 0) {
|
||||
strlcpy(udev_rules_filename, value, sizeof(udev_rules_filename));
|
||||
no_trailing_slash(udev_rules_filename);
|
||||
remove_trailing_char(udev_rules_filename, '/');
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -232,7 +232,7 @@ void udev_init_config(void)
|
|||
env = getenv("UDEV_CONFIG_FILE");
|
||||
if (env) {
|
||||
strlcpy(udev_config_filename, env, sizeof(udev_config_filename));
|
||||
no_trailing_slash(udev_config_filename);
|
||||
remove_trailing_char(udev_config_filename, '/');
|
||||
}
|
||||
|
||||
parse_config_file();
|
||||
|
|
|
|||
145
udev_rules.c
145
udev_rules.c
|
|
@ -43,7 +43,6 @@
|
|||
#include "udev_rules.h"
|
||||
#include "udev_db.h"
|
||||
|
||||
static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr);
|
||||
|
||||
/* compare string with pattern (supports * ? [0-9] [!A-Z]) */
|
||||
static int strcmp_pattern(const char *p, const char *s)
|
||||
|
|
@ -167,6 +166,33 @@ static int find_free_number(struct udevice *udev, const char *name)
|
|||
}
|
||||
}
|
||||
|
||||
static int find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device,
|
||||
const char *name, char *value, size_t len)
|
||||
{
|
||||
struct sysfs_attribute *tmpattr;
|
||||
|
||||
dbg("look for device attribute '%s'", name);
|
||||
if (class_dev) {
|
||||
tmpattr = sysfs_get_classdev_attr(class_dev, name);
|
||||
if (tmpattr)
|
||||
goto attr_found;
|
||||
}
|
||||
if (sysfs_device) {
|
||||
tmpattr = sysfs_get_device_attr(sysfs_device, name);
|
||||
if (tmpattr)
|
||||
goto attr_found;
|
||||
}
|
||||
|
||||
return -1;
|
||||
|
||||
attr_found:
|
||||
strlcpy(value, tmpattr->value, len);
|
||||
remove_trailing_char(value, '\n');
|
||||
|
||||
dbg("found attribute '%s'", tmpattr->path);
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void apply_format(struct udevice *udev, char *string, size_t maxsize,
|
||||
struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device)
|
||||
{
|
||||
|
|
@ -176,7 +202,6 @@ static void apply_format(struct udevice *udev, char *string, size_t maxsize,
|
|||
int len;
|
||||
int i;
|
||||
char c;
|
||||
struct sysfs_attribute *tmpattr;
|
||||
unsigned int next_free_number;
|
||||
struct sysfs_class_device *class_dev_parent;
|
||||
|
||||
|
|
@ -257,30 +282,21 @@ static void apply_format(struct udevice *udev, char *string, size_t maxsize,
|
|||
}
|
||||
break;
|
||||
case 's':
|
||||
if (!class_dev)
|
||||
break;
|
||||
if (attr == NULL) {
|
||||
dbg("missing attribute");
|
||||
break;
|
||||
}
|
||||
tmpattr = find_sysfs_attribute(class_dev, sysfs_device, attr);
|
||||
if (tmpattr == NULL) {
|
||||
if (find_sysfs_attribute(class_dev, sysfs_device, attr, temp2, sizeof(temp2)) != 0) {
|
||||
dbg("sysfs attribute '%s' not found", attr);
|
||||
break;
|
||||
}
|
||||
/* strip trailing whitespace of matching value */
|
||||
if (isspace(tmpattr->value[strlen(tmpattr->value)-1])) {
|
||||
i = len = strlen(tmpattr->value);
|
||||
while (i > 0 && isspace(tmpattr->value[i-1]))
|
||||
i--;
|
||||
if (i < len) {
|
||||
tmpattr->value[i] = '\0';
|
||||
dbg("remove %i trailing whitespace chars from '%s'",
|
||||
len - i, tmpattr->value);
|
||||
}
|
||||
}
|
||||
strlcat(string, tmpattr->value, maxsize);
|
||||
dbg("substitute sysfs value '%s'", tmpattr->value);
|
||||
/* strip trailing whitespace of sysfs value */
|
||||
i = strlen(temp2);
|
||||
while (i > 0 && isspace(temp2[i-1]))
|
||||
temp2[--i] = '\0';
|
||||
replace_untrusted_chars(temp2);
|
||||
strlcat(string, temp2, maxsize);
|
||||
dbg("substitute sysfs value '%s'", temp2);
|
||||
break;
|
||||
case '%':
|
||||
strlcat(string, "%", maxsize);
|
||||
|
|
@ -385,8 +401,7 @@ static int execute_program(struct udevice *udev, const char *path, char *value,
|
|||
pid = fork();
|
||||
switch(pid) {
|
||||
case 0:
|
||||
/* child */
|
||||
/* dup2 write side of pipe to STDOUT */
|
||||
/* child dup2 write side of pipe to STDOUT */
|
||||
dup2(fds[1], STDOUT_FILENO);
|
||||
retval = execv(arg, argv);
|
||||
|
||||
|
|
@ -402,7 +417,12 @@ static int execute_program(struct udevice *udev, const char *path, char *value,
|
|||
i = 0;
|
||||
while (1) {
|
||||
count = read(fds[0], value + i, len - i-1);
|
||||
if (count <= 0)
|
||||
if (count < 0) {
|
||||
err("read failed with '%s'", strerror(errno));
|
||||
retval = -1;
|
||||
}
|
||||
|
||||
if (count == 0)
|
||||
break;
|
||||
|
||||
i += count;
|
||||
|
|
@ -412,16 +432,7 @@ static int execute_program(struct udevice *udev, const char *path, char *value,
|
|||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (count < 0) {
|
||||
err("read failed with '%s'", strerror(errno));
|
||||
retval = -1;
|
||||
}
|
||||
|
||||
if (i > 0 && value[i-1] == '\n')
|
||||
i--;
|
||||
value[i] = '\0';
|
||||
dbg("result is '%s'", value);
|
||||
|
||||
close(fds[0]);
|
||||
waitpid(pid, &status, 0);
|
||||
|
|
@ -431,70 +442,38 @@ static int execute_program(struct udevice *udev, const char *path, char *value,
|
|||
retval = -1;
|
||||
}
|
||||
}
|
||||
|
||||
if (!retval) {
|
||||
remove_trailing_char(value, '\n');
|
||||
dbg("result is '%s'", value);
|
||||
replace_untrusted_chars(value);
|
||||
} else
|
||||
value[0] = '\0';
|
||||
|
||||
return retval;
|
||||
}
|
||||
|
||||
static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr)
|
||||
{
|
||||
struct sysfs_attribute *tmpattr = NULL;
|
||||
char *c;
|
||||
|
||||
dbg("look for device attribute '%s'", attr);
|
||||
/* try to find the attribute in the class device directory */
|
||||
tmpattr = sysfs_get_classdev_attr(class_dev, attr);
|
||||
if (tmpattr)
|
||||
goto attr_found;
|
||||
|
||||
/* look in the class device directory if present */
|
||||
if (sysfs_device) {
|
||||
tmpattr = sysfs_get_device_attr(sysfs_device, attr);
|
||||
if (tmpattr)
|
||||
goto attr_found;
|
||||
}
|
||||
|
||||
return NULL;
|
||||
|
||||
attr_found:
|
||||
c = strchr(tmpattr->value, '\n');
|
||||
if (c != NULL)
|
||||
c[0] = '\0';
|
||||
|
||||
dbg("found attribute '%s'", tmpattr->path);
|
||||
return tmpattr;
|
||||
}
|
||||
|
||||
static int compare_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, struct key_pair *pair)
|
||||
{
|
||||
struct sysfs_attribute *tmpattr;
|
||||
char value[VALUE_SIZE];
|
||||
int i;
|
||||
int len;
|
||||
|
||||
if ((pair == NULL) || (pair->name[0] == '\0') || (pair->value == '\0'))
|
||||
return -ENODEV;
|
||||
|
||||
tmpattr = find_sysfs_attribute(class_dev, sysfs_device, pair->name);
|
||||
if (tmpattr == NULL)
|
||||
return -ENODEV;
|
||||
if (find_sysfs_attribute(class_dev, sysfs_device, pair->name, value, sizeof(value)) != 0)
|
||||
return -1;
|
||||
|
||||
/* strip trailing whitespace of value, if not asked to match for it */
|
||||
if (! isspace(pair->value[strlen(pair->value)-1])) {
|
||||
i = len = strlen(tmpattr->value);
|
||||
while (i > 0 && isspace(tmpattr->value[i-1]))
|
||||
i--;
|
||||
if (i < len) {
|
||||
tmpattr->value[i] = '\0';
|
||||
dbg("remove %i trailing whitespace chars from '%s'",
|
||||
len - i, tmpattr->value);
|
||||
}
|
||||
if (!isspace(pair->value[strlen(pair->value)-1])) {
|
||||
i = strlen(value);
|
||||
while (i > 0 && isspace(value[i-1]))
|
||||
value[--i] = '\0';
|
||||
dbg("removed %i trailing whitespace chars from '%s'", strlen(value)-i, value);
|
||||
}
|
||||
|
||||
dbg("compare attribute '%s' value '%s' with '%s'",
|
||||
pair->name, tmpattr->value, pair->value);
|
||||
if (strcmp_pattern(pair->value, tmpattr->value) != 0)
|
||||
return -ENODEV;
|
||||
dbg("compare attribute '%s' value '%s' with '%s'", pair->name, value, pair->value);
|
||||
if (strcmp_pattern(pair->value, value) != 0)
|
||||
return -1;
|
||||
|
||||
dbg("found matching attribute '%s' with value '%s'",
|
||||
pair->name, pair->value);
|
||||
dbg("found matching attribute '%s' with value '%s'", pair->name, pair->value);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
|
|||
18
udev_utils.c
18
udev_utils.c
|
|
@ -50,7 +50,7 @@ int udev_init_device(struct udevice *udev, const char* devpath, const char *subs
|
|||
|
||||
if (devpath) {
|
||||
strlcpy(udev->devpath, devpath, sizeof(udev->devpath));
|
||||
no_trailing_slash(udev->devpath);
|
||||
remove_trailing_char(udev->devpath, '/');
|
||||
|
||||
if (strncmp(udev->devpath, "/block/", 7) == 0)
|
||||
udev->type = DEV_BLOCK;
|
||||
|
|
@ -228,12 +228,24 @@ size_t buf_get_line(const char *buf, size_t buflen, size_t cur)
|
|||
return count - cur;
|
||||
}
|
||||
|
||||
void no_trailing_slash(char *path)
|
||||
void replace_untrusted_chars(char *string)
|
||||
{
|
||||
size_t len;
|
||||
|
||||
for (len = 0; string[len] != '\0'; len++) {
|
||||
if (strchr(";,~\\()\'", string[len])) {
|
||||
info("replace '%c' in '%s'", string[len], string);
|
||||
string[len] = '_';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void remove_trailing_char(char *path, char c)
|
||||
{
|
||||
size_t len;
|
||||
|
||||
len = strlen(path);
|
||||
while (len > 0 && path[len-1] == '/')
|
||||
while (len > 0 && path[len-1] == c)
|
||||
path[--len] = '\0';
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -39,7 +39,8 @@ extern int unlink_secure(const char *filename);
|
|||
extern int file_map(const char *filename, char **buf, size_t *bufsize);
|
||||
extern void file_unmap(char *buf, size_t bufsize);
|
||||
extern size_t buf_get_line(const char *buf, size_t buflen, size_t cur);
|
||||
extern void no_trailing_slash(char *path);
|
||||
extern void remove_trailing_char(char *path, char c);
|
||||
extern void replace_untrusted_chars(char *string);
|
||||
extern int name_list_add(struct list_head *name_list, const char *name, int sort);
|
||||
extern int add_matching_files(struct list_head *name_list, const char *dirname, const char *suffix);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue