bug-tar
[Top][All Lists]
Advanced

[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

Attachment: 0001-selinux-h-avoid-double-free-after-getfilecon.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]