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?
