diff mbox series

[2/3] libselinux: bail out on path truncations

Message ID 20221109195640.60484-2-cgzones@googlemail.com (mailing list archive)
State Accepted
Commit d97c34efa5b9
Headers show
Series [1/3] libselinux: make use of strndup | expand

Commit Message

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

Comments

James Carter Nov. 9, 2022, 9:38 p.m. UTC | #1 
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
>
diff mbox series

Patch

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;