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 -- -- Matthew Thode
Attachment:
signature.asc
Description: OpenPGP digital 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.
