diff mbox series

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

Message ID 20221114193208.9413-1-cgzones@googlemail.com (mailing list archive)
State Accepted
Commit d31280c26e06
Headers show
Series None | expand

Commit Message

Christian Göttsche Nov. 14, 2022, 7:32 p.m. UTC
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(-)

Comments

James Carter Nov. 21, 2022, 9:39 p.m. UTC | #1
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
>
James Carter Nov. 23, 2022, 3:03 p.m. UTC | #2
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
> >
diff mbox series

Patch

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;