I haven't looked at the whole patch properly yet, but noting what I found so far (see below)... On Sat, Feb 8, 2020 at 8:36 AM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote: > get_ordered_context_list() code used to ask the kernel to compute the complete > set of reachable contexts using /sys/fs/selinux/user aka > security_compute_user(). This set can be so huge so that it doesn't fit into a > kernel page and security_compute_user() fails. Even if it doesn't fail, > get_ordered_context_list() throws away the vast majority of the returned > contexts because they don't match anything in > /etc/selinux/targeted/contexts/default_contexts or > /etc/selinux/targeted/contexts/users/ > > get_ordered_context_list() is rewritten to compute set of contexts based on > /etc/selinux/targeted/contexts/users/ and > /etc/selinux/targeted/contexts/default_contexts files and to return only valid > contexts, using security_check_context(), from this set. > > Fixes: https://github.com/SELinuxProject/selinux/issues/28 > > Signed-off-by: Petr Lautrbach <plautrba@xxxxxxxxxx> > --- > > v3 changes: > > - check return values of context_range_set() and context_str() > - do not add duplicate entries to reachable contexts > > > libselinux/src/get_context_list.c | 219 ++++++++++++++---------------- > 1 file changed, 103 insertions(+), 116 deletions(-) > > diff --git a/libselinux/src/get_context_list.c b/libselinux/src/get_context_list.c > index 689e46589f30..cc39f8c6a96c 100644 > --- a/libselinux/src/get_context_list.c > +++ b/libselinux/src/get_context_list.c <snip> > -static int get_context_order(FILE * fp, > +static int get_context_user(FILE * fp, > char * fromcon, > - char ** reachable, > - unsigned int nreach, > - unsigned int *ordering, unsigned int *nordered) > + const char * user, > + char ***reachable, > + unsigned int *nreachable) > { > char *start, *end = NULL; > char *line = NULL; > - size_t line_len = 0; > + size_t line_len = 0, usercon_len; > + size_t user_len = strlen(user); > ssize_t len; > int found = 0; > - const char *fromrole, *fromtype; > + const char *fromrole, *fromtype, *fromlevel; > char *linerole, *linetype; > - unsigned int i; > + char **new_reachable = NULL; > + char *usercon_str; > context_t con; > + context_t usercon; > + > int rc; > > errno = -EINVAL; > @@ -180,6 +158,7 @@ static int get_context_order(FILE * fp, > return -1; > fromrole = context_role_get(con); > fromtype = context_type_get(con); > + fromlevel = context_range_get(con); > if (!fromrole || !fromtype) { > context_free(con); > return -1; > @@ -243,23 +222,89 @@ static int get_context_order(FILE * fp, > if (*end) > *end++ = 0; > > - /* Check for a match in the reachable list. */ > - rc = find_partialcon(reachable, nreach, start); > - if (rc < 0) { > - /* No match, skip it. */ > + /* Check whether a new context is valid */ > + if (SIZE_MAX - user_len < strlen(start) + 1) { You need to also account for either the null character or the colon (not sure which one you missed). > + fprintf(stderr, "%s: one of partial contexts is too big\n", __FUNCTION__); > + errno = EINVAL; > + rc = -1; > + goto out; > + } > + usercon_len = user_len + strlen(start) + 1; > + usercon_str = malloc(usercon_len); Again, you are not accounting for one of '\0' or ':' here. > + if (!usercon_str) { > + rc = -1; > + goto out; > + } > + > + /* set range from fromcon in the new usercon */ > + snprintf(usercon_str, usercon_len - 1, "%s:%s", user, start); The second argument should be just usercon_len (assuming you fix the above) - see snprintf(3): "The functions snprintf() and vsnprintf() write at most size bytes (including the terminating null byte ('\0')) to str." So this should always result in a string with 2 last characters truncated (followed by a null character and an unused byte). Or am I missing something? Did you get correct strings when you tested this? <snip> -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
