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