All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joshua Brindle <brindle@quarksecurity.com>
To: Steve Lawrence <slawrence@tresys.com>
Cc: Petr Lautrbach <plautrba@redhat.com>, selinux@tycho.nsa.gov
Subject: Re: [PATCH] libselinux: Verify context input to funtions to make sure the context field is not null.
Date: Thu, 17 Dec 2015 09:22:54 -0500	[thread overview]
Message-ID: <5672C53E.30700@quarksecurity.com> (raw)
In-Reply-To: <5672BED4.6000802@tresys.com>

Steve Lawrence wrote:
> I believe this patch, or something similar, was sent to the list in the
> past and was rejected. Passing in a NULL context is considered invalid
> use, similar to strdup/strcmp/etc. and is a bug in the calling process.
>

It isn't unreasonable for an API to indicate invalid usage (which is 
what EINVAL does). The argument that strdup lets you shoot yourself in 
the foot so we should to isn't very compelling...

> On 12/09/2015 03:47 PM, Petr Lautrbach wrote:
>> From: Dan Walsh<dwalsh@redhat.com>
>>
>> Return errno EINVAL, to prevent segfault.
>>
>> Signed-off-by: Petr Lautrbach<plautrba@redhat.com>
>> ---
>>   libselinux/src/avc_sidtab.c           | 5 +++++
>>   libselinux/src/canonicalize_context.c | 5 +++++
>>   libselinux/src/check_context.c        | 5 +++++
>>   libselinux/src/compute_av.c           | 5 +++++
>>   libselinux/src/compute_create.c       | 5 +++++
>>   libselinux/src/compute_member.c       | 5 +++++
>>   libselinux/src/compute_relabel.c      | 5 +++++
>>   libselinux/src/compute_user.c         | 5 +++++
>>   libselinux/src/fsetfilecon.c          | 8 ++++++--
>>   libselinux/src/lsetfilecon.c          | 9 +++++++--
>>   libselinux/src/setfilecon.c           | 8 ++++++--
>>   11 files changed, 59 insertions(+), 6 deletions(-)
>>
>> diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c
>> index 9669264..a46cfa7 100644
>> --- a/libselinux/src/avc_sidtab.c
>> +++ b/libselinux/src/avc_sidtab.c
>> @@ -81,6 +81,11 @@ sidtab_context_to_sid(struct sidtab *s,
>>   	int hvalue, rc = 0;
>>   	struct sidtab_node *cur;
>>
>> +	if (! ctx) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	*sid = NULL;
>>   	hvalue = sidtab_hash(ctx);
>>
>> diff --git a/libselinux/src/canonicalize_context.c b/libselinux/src/canonicalize_context.c
>> index 7cf3139..9d8dc86 100644
>> --- a/libselinux/src/canonicalize_context.c
>> +++ b/libselinux/src/canonicalize_context.c
>> @@ -17,6 +17,11 @@ int security_canonicalize_context_raw(const char * con,
>>   	size_t size;
>>   	int fd, ret;
>>
>> +	if (! con) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	if (!selinux_mnt) {
>>   		errno = ENOENT;
>>   		return -1;
>> diff --git a/libselinux/src/check_context.c b/libselinux/src/check_context.c
>> index 52063fa..9637dd7 100644
>> --- a/libselinux/src/check_context.c
>> +++ b/libselinux/src/check_context.c
>> @@ -14,6 +14,11 @@ int security_check_context_raw(const char * con)
>>   	char path[PATH_MAX];
>>   	int fd, ret;
>>
>> +	if (! con) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	if (!selinux_mnt) {
>>   		errno = ENOENT;
>>   		return -1;
>> diff --git a/libselinux/src/compute_av.c b/libselinux/src/compute_av.c
>> index 937e5c3..e67b3d3 100644
>> --- a/libselinux/src/compute_av.c
>> +++ b/libselinux/src/compute_av.c
>> @@ -26,6 +26,11 @@ int security_compute_av_flags_raw(const char * scon,
>>   		return -1;
>>   	}
>>
>> +	if ((! scon) || (! tcon)) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	snprintf(path, sizeof path, "%s/access", selinux_mnt);
>>   	fd = open(path, O_RDWR);
>>   	if (fd<  0)
>> diff --git a/libselinux/src/compute_create.c b/libselinux/src/compute_create.c
>> index 9559d42..7de721a 100644
>> --- a/libselinux/src/compute_create.c
>> +++ b/libselinux/src/compute_create.c
>> @@ -64,6 +64,11 @@ int security_compute_create_name_raw(const char * scon,
>>   		return -1;
>>   	}
>>
>> +	if ((! scon) || (! tcon)) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	snprintf(path, sizeof path, "%s/create", selinux_mnt);
>>   	fd = open(path, O_RDWR);
>>   	if (fd<  0)
>> diff --git a/libselinux/src/compute_member.c b/libselinux/src/compute_member.c
>> index 1fc7e41..4ff2173 100644
>> --- a/libselinux/src/compute_member.c
>> +++ b/libselinux/src/compute_member.c
>> @@ -25,6 +25,11 @@ int security_compute_member_raw(const char * scon,
>>   		return -1;
>>   	}
>>
>> +	if ((! scon) || (! tcon)) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	snprintf(path, sizeof path, "%s/member", selinux_mnt);
>>   	fd = open(path, O_RDWR);
>>   	if (fd<  0)
>> diff --git a/libselinux/src/compute_relabel.c b/libselinux/src/compute_relabel.c
>> index 4615aee..6a37acf 100644
>> --- a/libselinux/src/compute_relabel.c
>> +++ b/libselinux/src/compute_relabel.c
>> @@ -25,6 +25,11 @@ int security_compute_relabel_raw(const char * scon,
>>   		return -1;
>>   	}
>>
>> +	if ((! scon) || (! tcon)) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	snprintf(path, sizeof path, "%s/relabel", selinux_mnt);
>>   	fd = open(path, O_RDWR);
>>   	if (fd<  0)
>> diff --git a/libselinux/src/compute_user.c b/libselinux/src/compute_user.c
>> index b37c5d3..3737c28 100644
>> --- a/libselinux/src/compute_user.c
>> +++ b/libselinux/src/compute_user.c
>> @@ -24,6 +24,11 @@ int security_compute_user_raw(const char * scon,
>>   		return -1;
>>   	}
>>
>> +	if (! scon) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>>   	snprintf(path, sizeof path, "%s/user", selinux_mnt);
>>   	fd = open(path, O_RDWR);
>>   	if (fd<  0)
>> diff --git a/libselinux/src/fsetfilecon.c b/libselinux/src/fsetfilecon.c
>> index 52707d0..83c138e 100644
>> --- a/libselinux/src/fsetfilecon.c
>> +++ b/libselinux/src/fsetfilecon.c
>> @@ -9,8 +9,12 @@
>>
>>   int fsetfilecon_raw(int fd, const char * context)
>>   {
>> -	int rc = fsetxattr(fd, XATTR_NAME_SELINUX, context, strlen(context) + 1,
>> -			 0);
>> +	int rc;
>> +	if (! context) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +	rc = fsetxattr(fd, XATTR_NAME_SELINUX, context, strlen(context) + 1, 0);
>>   	if (rc<  0&&  errno == ENOTSUP) {
>>   		char * ccontext = NULL;
>>   		int err = errno;
>> diff --git a/libselinux/src/lsetfilecon.c b/libselinux/src/lsetfilecon.c
>> index 1d3b28a..278e42a 100644
>> --- a/libselinux/src/lsetfilecon.c
>> +++ b/libselinux/src/lsetfilecon.c
>> @@ -9,8 +9,13 @@
>>
>>   int lsetfilecon_raw(const char *path, const char * context)
>>   {
>> -	int rc = lsetxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1,
>> -			 0);
>> +	int rc;
>> +	if (! context) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>> +	rc = lsetxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1, 0);
>>   	if (rc<  0&&  errno == ENOTSUP) {
>>   		char * ccontext = NULL;
>>   		int err = errno;
>> diff --git a/libselinux/src/setfilecon.c b/libselinux/src/setfilecon.c
>> index d05969c..dddce3c 100644
>> --- a/libselinux/src/setfilecon.c
>> +++ b/libselinux/src/setfilecon.c
>> @@ -9,8 +9,12 @@
>>
>>   int setfilecon_raw(const char *path, const char * context)
>>   {
>> -	int rc = setxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1,
>> -			0);
>> +	int rc;
>> +	if (! context) {
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +	rc = setxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1, 0);
>>   	if (rc<  0&&  errno == ENOTSUP) {
>>   		char * ccontext = NULL;
>>   		int err = errno;
>>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

      parent reply	other threads:[~2015-12-17 14:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-09 20:47 [PATCH] libselinux: Verify context input to funtions to make sure the context field is not null Petr Lautrbach
2015-12-17 13:55 ` Steve Lawrence
2015-12-17 14:06   ` Petr Lautrbach
2015-12-17 14:22   ` Joshua Brindle [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5672C53E.30700@quarksecurity.com \
    --to=brindle@quarksecurity.com \
    --cc=plautrba@redhat.com \
    --cc=selinux@tycho.nsa.gov \
    --cc=slawrence@tresys.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.