+cc gnulib mailing list
Thanks for reporting this, Ben.
On Thursday 16 of April 2015 13:25:59 Ben Shelton wrote:
> When SELinux is enabled in the kernel but no policy is loaded, files may
> be marked as unlabeled. When these files are processed,
> rpl_lgetfilecon() returns the security context as "unlabeled".
> map_to_failure() then frees the security context, sets errno to ENODATA,
> and returns -1. However, since the security context is not NULL,
> xattr_selinux_coder() attempts to read from it when the header is
> generated, which leads to memory corruption (and a failure on some
> future malloc).
>
> For unlabeled files, set the security context to NULL to avoid this
> use-after-free bug.
>
> Signed-off-by: Ben Shelton <ben.shel...@ni.com>
> ---
> src/xattrs.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/xattrs.c b/src/xattrs.c
> index 307ee38..0648c18 100644
> --- a/src/xattrs.c
> +++ b/src/xattrs.c
> @@ -551,6 +551,11 @@ xattrs_selinux_get (int parentfd, char const *file_name,
> fgetfilecon (fd, &st->cntx_name)
> : lgetfileconat (parentfd, file_name, &st->cntx_name);
>
> + /* If the file is unlabeled, map_to_failure() will have freed
> cntx_name.
> + * If this is the case, set it to NULL so it is not used after
> freeing. */
> + if (result == -1 && errno == ENODATA)
> + st->cntx_name = NULL;
> +
> if (result == -1 && errno != ENODATA && errno != ENOTSUP)
> call_arg_warn (fd ? "fgetfilecon" : "lgetfileconat", file_name);
> #endif
> --
> 2.3.2
Seems like we could safely set the pointer to NULL directly in
'map_to_failure()' gnulib function. This is not very precisely spelled in
getfilecon(3), but based on statement:
The returned context should be freed with freecon(3) if non-NULL.
.. seems like in this case *getfilecon() should rather return NULL in
context pointer rather than random freed pointer. Patch attached.
Pavel
>From 646ae813353ee8953eb0a4f6a06f4022015c151e Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Wed, 1 Jul 2015 12:30:57 +0200
Subject: [PATCH] selinux-h: avoid double free after *getfilecon()
Originally reported by Ben Shelton on bug-tar:
http://lists.gnu.org/archive/html/bug-tar/2015-04/msg00009.html
* lib/getfilecon.c (map_to_failure): Set the already freed '*con'
pointer to NULL. Man getfilecon(3) says that any non-NULL '*con'
parameter should be freed by freecon(3) (regardless the return
value).
---
lib/getfilecon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/getfilecon.c b/lib/getfilecon.c
index 2aa489e..86d2222 100644
--- a/lib/getfilecon.c
+++ b/lib/getfilecon.c
@@ -57,6 +57,7 @@ map_to_failure (int ret, security_context_t *con)
if (ret == 10 && strcmp (*con, "unlabeled") == 0)
{
freecon (*con);
+ *con = NULL;
errno = ENODATA;
return -1;
}
--
2.1.0
