* [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
* 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
* [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 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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.