| Message ID | 20221109195640.60484-3-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d31280c26e06 |
| Headers | show |
| Series | [1/3] libselinux: make use of strndup | expand |
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 >
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;
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(-)