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.
