[PATCH 1/3] libselinux: make use of strndup

[PATCH 1/3] libselinux: make use of strndup

[Christian Göttsche]

Using strndup(3) instead of malloc(3) followed by strncpy(3) simplifies
the code and pleases GCC:

    In file included from /usr/include/string.h:535,
                     from context.c:2:
    In function ‘strncpy’,
        inlined from ‘context_new’ at context.c:74:3:
    /usr/include/x86_64-linux-gnu/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ destination unchanged after copying no bytes [-Werror=stringop-truncation]
       95 |   return __builtin___strncpy_chk (__dest, __src, __len,
          |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       96 |                                   __glibc_objsize (__dest));
          |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/context.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libselinux/src/context.c b/libselinux/src/context.c
index 9dddbc5a..a6779a03 100644
--- a/libselinux/src/context.c
+++ b/libselinux/src/context.c
@@ -68,11 +68,9 @@ context_t context_new(const char *str)
 			for (p = tok; *p; p++) {	/* empty */
 			}
 		}
-		n->component[i] = (char *)malloc(p - tok + 1);
+		n->component[i] = strndup(tok, p - tok);
 		if (n->component[i] == 0)
 			goto err;
-		strncpy(n->component[i], tok, p - tok);
-		n->component[i][p - tok] = '\0';
 		tok = *p ? p + 1 : p;
 	}
 	return result;
-- 
2.38.1

[PATCH 2/3] libselinux: bail out on path truncations

[Christian Göttsche]

Bail out if computed paths based on user input are being truncated, to
avoid wrong files to be opened.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/booleans.c            |  4 ++--
 libselinux/src/get_initial_context.c |  8 ++++++--
 libselinux/src/stringrep.c           | 15 ++++++++++++---
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
index ef1f64a0..66c946f9 100644
--- a/libselinux/src/booleans.c
+++ b/libselinux/src/booleans.c
@@ -164,7 +164,7 @@ static int bool_open(const char *name, int flag) {
 		return -1;
 
 	ret = snprintf(fname, len, "%s%s%s", selinux_mnt, SELINUX_BOOL_DIR, name);
-	if (ret < 0)
+	if (ret < 0 || (size_t)ret >= len)
 		goto out;
 	assert(ret < len);
 
@@ -184,7 +184,7 @@ static int bool_open(const char *name, int flag) {
 	fname = ptr;
 
 	ret = snprintf(fname, len, "%s%s%s", selinux_mnt, SELINUX_BOOL_DIR, alt_name);
-	if (ret < 0)
+	if (ret < 0 || (size_t)ret >= len)
 		goto out;
 	assert(ret < len);
 
diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
index 97ae3dcf..87c8adfa 100644
--- a/libselinux/src/get_initial_context.c
+++ b/libselinux/src/get_initial_context.c
@@ -23,8 +23,12 @@ int security_get_initial_context_raw(const char * name, char ** con)
 		return -1;
 	}
 
-	snprintf(path, sizeof path, "%s%s%s", 
-		 selinux_mnt, SELINUX_INITCON_DIR, name);
+	ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
+	if (ret < 0 || (size_t)ret >= sizeof path) {
+		errno = EOVERFLOW;
+		return -1;
+	}
+
 	fd = open(path, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		return -1;
diff --git a/libselinux/src/stringrep.c b/libselinux/src/stringrep.c
index 592410e5..d2237d1c 100644
--- a/libselinux/src/stringrep.c
+++ b/libselinux/src/stringrep.c
@@ -82,7 +82,10 @@ static struct discover_class_node * discover_class(const char *s)
 		goto err2;
 
 	/* load up class index */
-	snprintf(path, sizeof path, "%s/class/%s/index", selinux_mnt,s);
+	ret = snprintf(path, sizeof path, "%s/class/%s/index", selinux_mnt,s);
+	if (ret < 0 || (size_t)ret >= sizeof path)
+		goto err3;
+
 	fd = open(path, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto err3;
@@ -97,7 +100,10 @@ static struct discover_class_node * discover_class(const char *s)
 		goto err3;
 
 	/* load up permission indices */
-	snprintf(path, sizeof path, "%s/class/%s/perms",selinux_mnt,s);
+	ret = snprintf(path, sizeof path, "%s/class/%s/perms",selinux_mnt,s);
+	if (ret < 0 || (size_t)ret >= sizeof path)
+		goto err3;
+
 	dir = opendir(path);
 	if (dir == NULL)
 		goto err3;
@@ -107,7 +113,10 @@ static struct discover_class_node * discover_class(const char *s)
 		unsigned int value;
 		struct stat m;
 
-		snprintf(path, sizeof path, "%s/class/%s/perms/%s", selinux_mnt,s,dentry->d_name);
+		ret = snprintf(path, sizeof path, "%s/class/%s/perms/%s", selinux_mnt,s,dentry->d_name);
+		if (ret < 0 || (size_t)ret >= sizeof path)
+			goto err4;
+
 		fd = open(path, O_RDONLY | O_CLOEXEC);
 		if (fd < 0)
 			goto err4;
-- 
2.38.1

[PATCH 3/3] libselinux: filter arguments with path separators

[Christian Göttsche]

Boolean names, taken by security_get_boolean_pending(3),
security_get_boolean_active(3) and security_set_boolean(3), as well as
user names, taken by security_get_initial_context(3), are used in path
constructions.  Ensure they do not contain path separators to avoid
unwanted path traversal.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/booleans.c            | 7 ++++++-
 libselinux/src/get_initial_context.c | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
index 66c946f9..64248191 100644
--- a/libselinux/src/booleans.c
+++ b/libselinux/src/booleans.c
@@ -152,7 +152,7 @@ static int bool_open(const char *name, int flag) {
 	int ret;
 	char *ptr;
 
-	if (!name) {
+	if (!name || strchr(name, '/')) {
 		errno = EINVAL;
 		return -1;
 	}
@@ -176,6 +176,11 @@ static int bool_open(const char *name, int flag) {
 	if (!alt_name)
 		goto out;
 
+	if (strchr(alt_name, '/')) {
+		errno = EINVAL;
+		goto out;
+	}
+
 	/* note the 'sizeof' gets us enough room for the '\0' */
 	len = strlen(alt_name) + strlen(selinux_mnt) + sizeof(SELINUX_BOOL_DIR);
 	ptr = realloc(fname, len);
diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
index 87c8adfa..0f25ba3f 100644
--- a/libselinux/src/get_initial_context.c
+++ b/libselinux/src/get_initial_context.c
@@ -23,6 +23,11 @@ int security_get_initial_context_raw(const char * name, char ** con)
 		return -1;
 	}
 
+	if (strchr(name, '/')) {
+		errno = EINVAL;
+		return -1;
+	}
+
 	ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
 	if (ret < 0 || (size_t)ret >= sizeof path) {
 		errno = EOVERFLOW;
-- 
2.38.1

Re: [PATCH 2/3] libselinux: bail out on path truncations

[James Carter]

On Wed, Nov 9, 2022 at 3:07 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Bail out if computed paths based on user input are being truncated, to
> avoid wrong files to be opened.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libselinux/src/booleans.c            |  4 ++--
>  libselinux/src/get_initial_context.c |  8 ++++++--
>  libselinux/src/stringrep.c           | 15 ++++++++++++---
>  3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> index ef1f64a0..66c946f9 100644
> --- a/libselinux/src/booleans.c
> +++ b/libselinux/src/booleans.c
> @@ -164,7 +164,7 @@ static int bool_open(const char *name, int flag) {
>                 return -1;
>
>         ret = snprintf(fname, len, "%s%s%s", selinux_mnt, SELINUX_BOOL_DIR, name);
> -       if (ret < 0)
> +       if (ret < 0 || (size_t)ret >= len)

The above causes the following error:

booleans.c:167:36: error: comparison of integer expressions of
different signedness: ‘long unsigned int’ and ‘int’
[-Werror=sign-compare]

>                 goto out;
>         assert(ret < len);
>
> @@ -184,7 +184,7 @@ static int bool_open(const char *name, int flag) {
>         fname = ptr;
>
>         ret = snprintf(fname, len, "%s%s%s", selinux_mnt, SELINUX_BOOL_DIR, alt_name);
> -       if (ret < 0)
> +       if (ret < 0 || (size_t)ret >= len)

Same here:

booleans.c:192:36: error: comparison of integer expressions of
different signedness: ‘long unsigned int’ and ‘int’
[-Werror=sign-compare]

Thanks,
Jim


>                 goto out;
>         assert(ret < len);
>
> diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
> index 97ae3dcf..87c8adfa 100644
> --- a/libselinux/src/get_initial_context.c
> +++ b/libselinux/src/get_initial_context.c
> @@ -23,8 +23,12 @@ int security_get_initial_context_raw(const char * name, char ** con)
>                 return -1;
>         }
>
> -       snprintf(path, sizeof path, "%s%s%s",
> -                selinux_mnt, SELINUX_INITCON_DIR, name);
> +       ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
> +       if (ret < 0 || (size_t)ret >= sizeof path) {
> +               errno = EOVERFLOW;
> +               return -1;
> +       }
> +
>         fd = open(path, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 return -1;
> diff --git a/libselinux/src/stringrep.c b/libselinux/src/stringrep.c
> index 592410e5..d2237d1c 100644
> --- a/libselinux/src/stringrep.c
> +++ b/libselinux/src/stringrep.c
> @@ -82,7 +82,10 @@ static struct discover_class_node * discover_class(const char *s)
>                 goto err2;
>
>         /* load up class index */
> -       snprintf(path, sizeof path, "%s/class/%s/index", selinux_mnt,s);
> +       ret = snprintf(path, sizeof path, "%s/class/%s/index", selinux_mnt,s);
> +       if (ret < 0 || (size_t)ret >= sizeof path)
> +               goto err3;
> +
>         fd = open(path, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 goto err3;
> @@ -97,7 +100,10 @@ static struct discover_class_node * discover_class(const char *s)
>                 goto err3;
>
>         /* load up permission indices */
> -       snprintf(path, sizeof path, "%s/class/%s/perms",selinux_mnt,s);
> +       ret = snprintf(path, sizeof path, "%s/class/%s/perms",selinux_mnt,s);
> +       if (ret < 0 || (size_t)ret >= sizeof path)
> +               goto err3;
> +
>         dir = opendir(path);
>         if (dir == NULL)
>                 goto err3;
> @@ -107,7 +113,10 @@ static struct discover_class_node * discover_class(const char *s)
>                 unsigned int value;
>                 struct stat m;
>
> -               snprintf(path, sizeof path, "%s/class/%s/perms/%s", selinux_mnt,s,dentry->d_name);
> +               ret = snprintf(path, sizeof path, "%s/class/%s/perms/%s", selinux_mnt,s,dentry->d_name);
> +               if (ret < 0 || (size_t)ret >= sizeof path)
> +                       goto err4;
> +
>                 fd = open(path, O_RDONLY | O_CLOEXEC);
>                 if (fd < 0)
>                         goto err4;
> --
> 2.38.1
>

[PATCH v2 2/3] libselinux: bail out on path truncations

[Christian Göttsche]

Bail out if computed paths based on user input are being truncated, to
avoid wrong files to be opened.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
  - drop now obsolete assert(3) statements
  - use size_t as type for length variables
---
 libselinux/src/booleans.c            |  9 +++------
 libselinux/src/get_initial_context.c |  8 ++++++--
 libselinux/src/stringrep.c           | 15 ++++++++++++---
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
index ef1f64a0..dbcccd70 100644
--- a/libselinux/src/booleans.c
+++ b/libselinux/src/booleans.c
@@ -7,7 +7,6 @@
 
 #ifndef DISABLE_BOOL
 
-#include <assert.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -147,7 +146,7 @@ out:
 static int bool_open(const char *name, int flag) {
 	char *fname = NULL;
 	char *alt_name = NULL;
-	int len;
+	size_t len;
 	int fd = -1;
 	int ret;
 	char *ptr;
@@ -164,9 +163,8 @@ static int bool_open(const char *name, int flag) {
 		return -1;
 
 	ret = snprintf(fname, len, "%s%s%s", selinux_mnt, SELINUX_BOOL_DIR, name);
-	if (ret < 0)
+	if (ret < 0 || (size_t)ret >= len)
 		goto out;
-	assert(ret < len);
 
 	fd = open(fname, flag);
 	if (fd >= 0 || errno != ENOENT)
@@ -184,9 +182,8 @@ static int bool_open(const char *name, int flag) {
 	fname = ptr;
 
 	ret = snprintf(fname, len, "%s%s%s", selinux_mnt, SELINUX_BOOL_DIR, alt_name);
-	if (ret < 0)
+	if (ret < 0 || (size_t)ret >= len)
 		goto out;
-	assert(ret < len);
 
 	fd = open(fname, flag);
 out:
diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
index 97ae3dcf..87c8adfa 100644
--- a/libselinux/src/get_initial_context.c
+++ b/libselinux/src/get_initial_context.c
@@ -23,8 +23,12 @@ int security_get_initial_context_raw(const char * name, char ** con)
 		return -1;
 	}
 
-	snprintf(path, sizeof path, "%s%s%s", 
-		 selinux_mnt, SELINUX_INITCON_DIR, name);
+	ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
+	if (ret < 0 || (size_t)ret >= sizeof path) {
+		errno = EOVERFLOW;
+		return -1;
+	}
+
 	fd = open(path, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		return -1;
diff --git a/libselinux/src/stringrep.c b/libselinux/src/stringrep.c
index 592410e5..d2237d1c 100644
--- a/libselinux/src/stringrep.c
+++ b/libselinux/src/stringrep.c
@@ -82,7 +82,10 @@ static struct discover_class_node * discover_class(const char *s)
 		goto err2;
 
 	/* load up class index */
-	snprintf(path, sizeof path, "%s/class/%s/index", selinux_mnt,s);
+	ret = snprintf(path, sizeof path, "%s/class/%s/index", selinux_mnt,s);
+	if (ret < 0 || (size_t)ret >= sizeof path)
+		goto err3;
+
 	fd = open(path, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto err3;
@@ -97,7 +100,10 @@ static struct discover_class_node * discover_class(const char *s)
 		goto err3;
 
 	/* load up permission indices */
-	snprintf(path, sizeof path, "%s/class/%s/perms",selinux_mnt,s);
+	ret = snprintf(path, sizeof path, "%s/class/%s/perms",selinux_mnt,s);
+	if (ret < 0 || (size_t)ret >= sizeof path)
+		goto err3;
+
 	dir = opendir(path);
 	if (dir == NULL)
 		goto err3;
@@ -107,7 +113,10 @@ static struct discover_class_node * discover_class(const char *s)
 		unsigned int value;
 		struct stat m;
 
-		snprintf(path, sizeof path, "%s/class/%s/perms/%s", selinux_mnt,s,dentry->d_name);
+		ret = snprintf(path, sizeof path, "%s/class/%s/perms/%s", selinux_mnt,s,dentry->d_name);
+		if (ret < 0 || (size_t)ret >= sizeof path)
+			goto err4;
+
 		fd = open(path, O_RDONLY | O_CLOEXEC);
 		if (fd < 0)
 			goto err4;
-- 
2.38.1

Re: [PATCH 3/3] libselinux: filter arguments with path separators

[James Carter]

On Wed, Nov 9, 2022 at 3:07 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Boolean names, taken by security_get_boolean_pending(3),
> security_get_boolean_active(3) and security_set_boolean(3), as well as
> user names, taken by security_get_initial_context(3), are used in path
> constructions.  Ensure they do not contain path separators to avoid
> unwanted path traversal.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libselinux/src/booleans.c            | 7 ++++++-
>  libselinux/src/get_initial_context.c | 5 +++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> index 66c946f9..64248191 100644
> --- a/libselinux/src/booleans.c
> +++ b/libselinux/src/booleans.c
> @@ -152,7 +152,7 @@ static int bool_open(const char *name, int flag) {
>         int ret;
>         char *ptr;
>
> -       if (!name) {
> +       if (!name || strchr(name, '/')) {
>                 errno = EINVAL;
>                 return -1;
>         }
> @@ -176,6 +176,11 @@ static int bool_open(const char *name, int flag) {
>         if (!alt_name)
>                 goto out;
>
> +       if (strchr(alt_name, '/')) {
> +               errno = EINVAL;
> +               goto out;
> +       }
> +

I don't think that this check is needed. You have already checked name
by this point and it is reading booleans.subs_dist which is in the
/etc/selinux directory.
Besides, if it was going to be checked, it should be in the
selinux_boolean_sub() function.

Thanks,
Jim


>         /* note the 'sizeof' gets us enough room for the '\0' */
>         len = strlen(alt_name) + strlen(selinux_mnt) + sizeof(SELINUX_BOOL_DIR);
>         ptr = realloc(fname, len);
> diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
> index 87c8adfa..0f25ba3f 100644
> --- a/libselinux/src/get_initial_context.c
> +++ b/libselinux/src/get_initial_context.c
> @@ -23,6 +23,11 @@ int security_get_initial_context_raw(const char * name, char ** con)
>                 return -1;
>         }
>
> +       if (strchr(name, '/')) {
> +               errno = EINVAL;
> +               return -1;
> +       }
> +
>         ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
>         if (ret < 0 || (size_t)ret >= sizeof path) {
>                 errno = EOVERFLOW;
> --
> 2.38.1
>

[PATCH v3 3/3] libselinux: filter arguments with path separators

[Christian Göttsche]

Boolean names, taken by security_get_boolean_pending(3),
security_get_boolean_active(3) and security_set_boolean(3), as well as
user names, taken by security_get_initial_context(3), are used in path
constructions.  Ensure they do not contain path separators to avoid
unwanted path traversal.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
  - move check for translated boolean name into selinux_boolean_sub()
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/booleans.c            | 5 +++--
 libselinux/src/get_initial_context.c | 5 +++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
index dbcccd70..e34b39ff 100644
--- a/libselinux/src/booleans.c
+++ b/libselinux/src/booleans.c
@@ -131,7 +131,8 @@ char *selinux_boolean_sub(const char *name)
 			ptr++;
 		*ptr = '\0';
 
-		sub = strdup(dst);
+		if (!strchr(dst, '/'))
+			sub = strdup(dst);
 
 		break;
 	}
@@ -151,7 +152,7 @@ static int bool_open(const char *name, int flag) {
 	int ret;
 	char *ptr;
 
-	if (!name) {
+	if (!name || strchr(name, '/')) {
 		errno = EINVAL;
 		return -1;
 	}
diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
index 87c8adfa..0f25ba3f 100644
--- a/libselinux/src/get_initial_context.c
+++ b/libselinux/src/get_initial_context.c
@@ -23,6 +23,11 @@ int security_get_initial_context_raw(const char * name, char ** con)
 		return -1;
 	}
 
+	if (strchr(name, '/')) {
+		errno = EINVAL;
+		return -1;
+	}
+
 	ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
 	if (ret < 0 || (size_t)ret >= sizeof path) {
 		errno = EOVERFLOW;
-- 
2.38.1

Re: [PATCH v3 3/3] libselinux: filter arguments with path separators

[James Carter]

On Mon, Nov 14, 2022 at 2:36 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Boolean names, taken by security_get_boolean_pending(3),
> security_get_boolean_active(3) and security_set_boolean(3), as well as
> user names, taken by security_get_initial_context(3), are used in path
> constructions.  Ensure they do not contain path separators to avoid
> unwanted path traversal.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

For this series of patches (v1 for patch 1, v2 for patch 2, and this
v3 for patch 3):
Acked-by: James Carter <jwcart2@gmail.com>

> ---
> v3:
>   - move check for translated boolean name into selinux_boolean_sub()
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libselinux/src/booleans.c            | 5 +++--
>  libselinux/src/get_initial_context.c | 5 +++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> index dbcccd70..e34b39ff 100644
> --- a/libselinux/src/booleans.c
> +++ b/libselinux/src/booleans.c
> @@ -131,7 +131,8 @@ char *selinux_boolean_sub(const char *name)
>                         ptr++;
>                 *ptr = '\0';
>
> -               sub = strdup(dst);
> +               if (!strchr(dst, '/'))
> +                       sub = strdup(dst);
>
>                 break;
>         }
> @@ -151,7 +152,7 @@ static int bool_open(const char *name, int flag) {
>         int ret;
>         char *ptr;
>
> -       if (!name) {
> +       if (!name || strchr(name, '/')) {
>                 errno = EINVAL;
>                 return -1;
>         }
> diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
> index 87c8adfa..0f25ba3f 100644
> --- a/libselinux/src/get_initial_context.c
> +++ b/libselinux/src/get_initial_context.c
> @@ -23,6 +23,11 @@ int security_get_initial_context_raw(const char * name, char ** con)
>                 return -1;
>         }
>
> +       if (strchr(name, '/')) {
> +               errno = EINVAL;
> +               return -1;
> +       }
> +
>         ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
>         if (ret < 0 || (size_t)ret >= sizeof path) {
>                 errno = EOVERFLOW;
> --
> 2.38.1
>

Re: [PATCH v3 3/3] libselinux: filter arguments with path separators

[James Carter]

On Mon, Nov 21, 2022 at 4:39 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Mon, Nov 14, 2022 at 2:36 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Boolean names, taken by security_get_boolean_pending(3),
> > security_get_boolean_active(3) and security_set_boolean(3), as well as
> > user names, taken by security_get_initial_context(3), are used in path
> > constructions.  Ensure they do not contain path separators to avoid
> > unwanted path traversal.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> For this series of patches (v1 for patch 1, v2 for patch 2, and this
> v3 for patch 3):
> Acked-by: James Carter <jwcart2@gmail.com>
>
These three patches have been merged.
Thanks,
Jim

> > ---
> > v3:
> >   - move check for translated boolean name into selinux_boolean_sub()
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  libselinux/src/booleans.c            | 5 +++--
> >  libselinux/src/get_initial_context.c | 5 +++++
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> > index dbcccd70..e34b39ff 100644
> > --- a/libselinux/src/booleans.c
> > +++ b/libselinux/src/booleans.c
> > @@ -131,7 +131,8 @@ char *selinux_boolean_sub(const char *name)
> >                         ptr++;
> >                 *ptr = '\0';
> >
> > -               sub = strdup(dst);
> > +               if (!strchr(dst, '/'))
> > +                       sub = strdup(dst);
> >
> >                 break;
> >         }
> > @@ -151,7 +152,7 @@ static int bool_open(const char *name, int flag) {
> >         int ret;
> >         char *ptr;
> >
> > -       if (!name) {
> > +       if (!name || strchr(name, '/')) {
> >                 errno = EINVAL;
> >                 return -1;
> >         }
> > diff --git a/libselinux/src/get_initial_context.c b/libselinux/src/get_initial_context.c
> > index 87c8adfa..0f25ba3f 100644
> > --- a/libselinux/src/get_initial_context.c
> > +++ b/libselinux/src/get_initial_context.c
> > @@ -23,6 +23,11 @@ int security_get_initial_context_raw(const char * name, char ** con)
> >                 return -1;
> >         }
> >
> > +       if (strchr(name, '/')) {
> > +               errno = EINVAL;
> > +               return -1;
> > +       }
> > +
> >         ret = snprintf(path, sizeof path, "%s%s%s", selinux_mnt, SELINUX_INITCON_DIR, name);
> >         if (ret < 0 || (size_t)ret >= sizeof path) {
> >                 errno = EOVERFLOW;
> > --
> > 2.38.1
> >