Re: Bug in libselinux/src/setrans_client.c

[Francis Cunnane <frankc@xxxxxxxxxxxxxxxx>]

What do you propose....  This is free software.... Don't be a Jew.

  On 12/30/2013 7:11 PM, Matthew Thode wrote:

On 12/30/2013 10:11 AM, Stephen Smalley wrote:

Calling *setfilecon() with a NULL context is a bug in the caller, just
like calling strlen() with a NULL string.
Fix the callers, please.

On Wed, Dec 25, 2013 at 9:36 AM, Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:

2013/12/23 Daniel J Walsh wrote:

On 12/21/2013 09:27 AM, Nicolas Iooss wrote:

My first message was not so clear. The check in
libselinux/src/lsetfilecon.c line 35 [1] doesn't work because
selinux_trans_to_raw_context(context, &rcontext) returns 0 and sets
rcontext to NULL. This is why I'm asking to change the return value to
something else if you want "cp -a" working. This fix is not to introduce a
new feature but to fix an existing one.

Nicolas

How about if we add a check on lsetfilecon_raw?  Changing the behaviour on
selinux_trans_to_raw_context might cause other problems.

I agree. I've found
http://selinuxproject.org/page/LibselinuxAPISummary which says
precisely for selinux_trans_to_raw_context: "If passed NULL, sets the
returned context to NULL and returns 0." As this feature is
documented, callers may rely on it and changing this behavior is
likely to break things.

Moreover setfilecon_raw and fsetfilecon_raw have the same NULL-pointer
dereference issue. Do these functions need a patch too?

By the way, other callers of selinux_trans_to_raw_context may also
share this bug: avc_context_to_sid, security_canonicalize_context,
security_check_context, etc. Is doing a segmentation fault the
expected way to tell the caller it used a NULL pointer and should have
manually checked every parameter before calling any libselinux
function?

Thanks and merry Christmas!

Nicolas

diff --git a/libselinux/src/lsetfilecon.c b/libselinux/src/lsetfilecon.c
index 461e3f7..af3775e 100644
- --- a/libselinux/src/lsetfilecon.c
+++ b/libselinux/src/lsetfilecon.c
@@ -9,6 +9,10 @@

 int lsetfilecon_raw(const char *path, const security_context_t context)
 {
+       if (! context) {
+               errno=EINVAL;
+               return -1;
+       }
        return lsetxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1
                         0);
 }

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

I think I may have hit this bug as well.

https://bugs.gentoo.org/show_bug.cgi?id=495274

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.