flask: fix reading strings from guest memory Since the string size is being specified by the guest, we must range check it properly before doing allocations based on it. While for the two cases that are exposed only to trusted guests (via policy restriction) this just uses an arbitrary upper limit (PAGE_SIZE), for the FLASK_[GS]ETBOOL case (which any guest can use) the upper limit gets enforced based on the longest name across all boolean settings. This is XSA-84. Reported-by: Matthew Daley Signed-off-by: Jan Beulich Acked-by: Daniel De Graaf --- a/xen/xsm/flask/flask_op.c +++ b/xen/xsm/flask/flask_op.c @@ -53,6 +53,7 @@ static DEFINE_SPINLOCK(sel_sem); /* global data for booleans */ static int bool_num = 0; static int *bool_pending_values = NULL; +static size_t bool_maxstr; static int flask_security_make_bools(void); extern int ss_initialized; @@ -71,9 +72,15 @@ static int domain_has_security(struct do perms, NULL); } -static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf, uint32_t size) +static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf, + size_t size, size_t max_size) { - char *tmp = xmalloc_bytes(size + 1); + char *tmp; + + if ( size > max_size ) + return -ENOENT; + + tmp = xmalloc_array(char, size + 1); if ( !tmp ) return -ENOMEM; @@ -99,7 +106,7 @@ static int flask_security_user(struct xe if ( rv ) return rv; - rv = flask_copyin_string(arg->u.user, &user, arg->size); + rv = flask_copyin_string(arg->u.user, &user, arg->size, PAGE_SIZE); if ( rv ) return rv; @@ -210,7 +217,7 @@ static int flask_security_context(struct if ( rv ) return rv; - rv = flask_copyin_string(arg->context, &buf, arg->size); + rv = flask_copyin_string(arg->context, &buf, arg->size, PAGE_SIZE); if ( rv ) return rv; @@ -303,7 +310,7 @@ static int flask_security_resolve_bool(s if ( arg->bool_id != -1 ) return 0; - rv = flask_copyin_string(arg->name, &name, arg->size); + rv = flask_copyin_string(arg->name, &name, arg->size, bool_maxstr); if ( rv ) return rv; @@ -334,7 +341,7 @@ static int flask_security_set_bool(struc int num; int *values; - rv = security_get_bools(&num, NULL, &values); + rv = security_get_bools(&num, NULL, &values, NULL); if ( rv != 0 ) goto out; @@ -440,7 +447,7 @@ static int flask_security_make_bools(voi xfree(bool_pending_values); - ret = security_get_bools(&num, NULL, &values); + ret = security_get_bools(&num, NULL, &values, &bool_maxstr); if ( ret != 0 ) goto out; --- a/xen/xsm/flask/include/conditional.h +++ b/xen/xsm/flask/include/conditional.h @@ -13,7 +13,9 @@ #ifndef _FLASK_CONDITIONAL_H_ #define _FLASK_CONDITIONAL_H_ -int security_get_bools(int *len, char ***names, int **values); +#include + +int security_get_bools(int *len, char ***names, int **values, size_t *maxstr); int security_set_bools(int len, int *values); --- a/xen/xsm/flask/ss/services.c +++ b/xen/xsm/flask/ss/services.c @@ -1900,7 +1900,7 @@ int security_find_bool(const char *name) return rv; } -int security_get_bools(int *len, char ***names, int **values) +int security_get_bools(int *len, char ***names, int **values, size_t *maxstr) { int i, rc = -ENOMEM; @@ -1908,6 +1908,8 @@ int security_get_bools(int *len, char ** if ( names ) *names = NULL; *values = NULL; + if ( maxstr ) + *maxstr = 0; *len = policydb.p_bools.nprim; if ( !*len ) @@ -1929,16 +1931,17 @@ int security_get_bools(int *len, char ** for ( i = 0; i < *len; i++ ) { - size_t name_len; + size_t name_len = strlen(policydb.p_bool_val_to_name[i]); + (*values)[i] = policydb.bool_val_to_struct[i]->state; if ( names ) { - name_len = strlen(policydb.p_bool_val_to_name[i]) + 1; - (*names)[i] = (char*)xmalloc_array(char, name_len); + (*names)[i] = xmalloc_array(char, name_len + 1); if ( !(*names)[i] ) goto err; - strlcpy((*names)[i], policydb.p_bool_val_to_name[i], name_len); - (*names)[i][name_len - 1] = 0; + strlcpy((*names)[i], policydb.p_bool_val_to_name[i], name_len + 1); } + if ( maxstr && name_len > *maxstr ) + *maxstr = name_len; } rc = 0; out: @@ -2056,7 +2059,7 @@ static int security_preserve_bools(struc struct cond_bool_datum *booldatum; struct cond_node *cur; - rc = security_get_bools(&nbools, &bnames, &bvalues); + rc = security_get_bools(&nbools, &bnames, &bvalues, NULL); if ( rc ) goto out; for ( i = 0; i < nbools; i++ )