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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 9, 2022 at 3:07 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
>  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
>




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux