[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-tar] [PATCH RESEND] xattrs: Fix bug with --selinux option and u
From: |
Pavel Raiskup |
Subject: |
Re: [Bug-tar] [PATCH RESEND] xattrs: Fix bug with --selinux option and unlabeled files |
Date: |
Wed, 01 Jul 2015 12:49:35 +0200 |
User-agent: |
KMail/4.14.9 (Linux/4.0.5-200.fc21.x86_64+debug; KDE/4.14.9; x86_64; ; ) |
+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 <address@hidden>
> ---
> 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
0001-selinux-h-avoid-double-free-after-getfilecon.patch
Description: Text Data
- Re: [Bug-tar] [PATCH RESEND] xattrs: Fix bug with --selinux option and unlabeled files,
Pavel Raiskup <=