* [PATCH 1/3] libselinux: make use of strndup
@ 2022-11-09 19:56 Christian Göttsche
2022-11-09 19:56 ` [PATCH 2/3] libselinux: bail out on path truncations Christian Göttsche
2022-11-09 19:56 ` [PATCH 3/3] libselinux: filter arguments with path separators Christian Göttsche
0 siblings, 2 replies; 9+ messages in thread
From: Christian Göttsche @ 2022-11-09 19:56 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] libselinux: bail out on path truncations
2022-11-09 19:56 [PATCH 1/3] libselinux: make use of strndup Christian Göttsche
@ 2022-11-09 19:56 ` Christian Göttsche
2022-11-09 21:38 ` James Carter
2022-11-10 18:23 ` [PATCH v2 " Christian Göttsche
2022-11-09 19:56 ` [PATCH 3/3] libselinux: filter arguments with path separators Christian Göttsche
1 sibling, 2 replies; 9+ messages in thread
From: Christian Göttsche @ 2022-11-09 19:56 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] libselinux: filter arguments with path separators
2022-11-09 19:56 [PATCH 1/3] libselinux: make use of strndup Christian Göttsche
2022-11-09 19:56 ` [PATCH 2/3] libselinux: bail out on path truncations Christian Göttsche
@ 2022-11-09 19:56 ` Christian Göttsche
2022-11-10 21:28 ` James Carter
2022-11-14 19:32 ` [PATCH v3 " Christian Göttsche
1 sibling, 2 replies; 9+ messages in thread
From: Christian Göttsche @ 2022-11-09 19:56 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] libselinux: bail out on path truncations
2022-11-09 19:56 ` [PATCH 2/3] libselinux: bail out on path truncations Christian Göttsche
@ 2022-11-09 21:38 ` James Carter
2022-11-10 18:23 ` [PATCH v2 " Christian Göttsche
1 sibling, 0 replies; 9+ messages in thread
From: James Carter @ 2022-11-09 21:38 UTC (permalink / raw)
To: Christian Göttsche; +Cc: selinux
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] libselinux: bail out on path truncations
2022-11-09 19:56 ` [PATCH 2/3] libselinux: bail out on path truncations Christian Göttsche
2022-11-09 21:38 ` James Carter
@ 2022-11-10 18:23 ` Christian Göttsche
1 sibling, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2022-11-10 18:23 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] libselinux: filter arguments with path separators
2022-11-09 19:56 ` [PATCH 3/3] libselinux: filter arguments with path separators Christian Göttsche
@ 2022-11-10 21:28 ` James Carter
2022-11-14 19:32 ` [PATCH v3 " Christian Göttsche
1 sibling, 0 replies; 9+ messages in thread
From: James Carter @ 2022-11-10 21:28 UTC (permalink / raw)
To: Christian Göttsche; +Cc: selinux
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] libselinux: filter arguments with path separators
2022-11-09 19:56 ` [PATCH 3/3] libselinux: filter arguments with path separators Christian Göttsche
2022-11-10 21:28 ` James Carter
@ 2022-11-14 19:32 ` Christian Göttsche
2022-11-21 21:39 ` James Carter
1 sibling, 1 reply; 9+ messages in thread
From: Christian Göttsche @ 2022-11-14 19:32 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] libselinux: filter arguments with path separators
2022-11-14 19:32 ` [PATCH v3 " Christian Göttsche
@ 2022-11-21 21:39 ` James Carter
2022-11-23 15:03 ` James Carter
0 siblings, 1 reply; 9+ messages in thread
From: James Carter @ 2022-11-21 21:39 UTC (permalink / raw)
To: Christian Göttsche; +Cc: selinux
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] libselinux: filter arguments with path separators
2022-11-21 21:39 ` James Carter
@ 2022-11-23 15:03 ` James Carter
0 siblings, 0 replies; 9+ messages in thread
From: James Carter @ 2022-11-23 15:03 UTC (permalink / raw)
To: Christian Göttsche; +Cc: selinux
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
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-23 15:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-09 19:56 [PATCH 1/3] libselinux: make use of strndup Christian Göttsche
2022-11-09 19:56 ` [PATCH 2/3] libselinux: bail out on path truncations Christian Göttsche
2022-11-09 21:38 ` James Carter
2022-11-10 18:23 ` [PATCH v2 " Christian Göttsche
2022-11-09 19:56 ` [PATCH 3/3] libselinux: filter arguments with path separators Christian Göttsche
2022-11-10 21:28 ` James Carter
2022-11-14 19:32 ` [PATCH v3 " Christian Göttsche
2022-11-21 21:39 ` James Carter
2022-11-23 15:03 ` James Carter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).