From 55fe40ac77b2a419cd9ade9de07e06d31ca18c5c Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Wed, 17 May 2017 22:51:45 +0200 Subject: [PATCH] libselinux: rework selabel_subs_init() to avoid use-after-free In selabel_subs_init(), when digest_add_specfile() fails, the returned value is a pointer to data which has been freed (because label "err" frees variable "sub" which is equals to the returned variable, "list"). Moreover since since commit fd56c5230cea ("Separate out the calling of local subs and dist subs in selabel_sub"), argument "list" of selabel_subs_init() has always been NULL (rec->subs and rec->dist_subs are both initialized to NULL in selabel_open() before selabel_file_init() is called). Drop selabel_file_init()'s "list" argument and free all the list items which have been allocated in this function, when the code encounters an error. Signed-off-by: Nicolas Iooss --- libselinux/src/label.c | 14 +++++++++++--- libselinux/src/label_file.c | 10 ++++------ libselinux/src/label_internal.h | 1 - 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/libselinux/src/label.c b/libselinux/src/label.c index 71a273bd..70f6809e 100644 --- a/libselinux/src/label.c +++ b/libselinux/src/label.c @@ -92,12 +92,11 @@ static char *selabel_sub(struct selabel_sub *ptr, const char *src) } struct selabel_sub *selabel_subs_init(const char *path, - struct selabel_sub *list, - struct selabel_digest *digest) + struct selabel_digest *digest) { char buf[1024]; FILE *cfg = fopen(path, "re"); - struct selabel_sub *sub = NULL; + struct selabel_sub *list = NULL, *sub = NULL; struct stat sb; if (!cfg) @@ -146,6 +145,7 @@ struct selabel_sub *selabel_subs_init(const char *path, sub->slen = strlen(src); sub->next = list; list = sub; + sub = NULL; } if (digest_add_specfile(digest, cfg, NULL, sb.st_size, path) < 0) @@ -158,6 +158,14 @@ err: if (sub) free(sub->src); free(sub); + while (list) { + sub = list->next; + free(list->src); + free(list->dst); + free(list); + list = sub; + } + list = NULL; goto out; } diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c index 0d4029bb..3ff75903 100644 --- a/libselinux/src/label_file.c +++ b/libselinux/src/label_file.c @@ -589,17 +589,15 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, if (!path) { rec->dist_subs = selabel_subs_init(selinux_file_context_subs_dist_path(), - rec->dist_subs, rec->digest); + rec->digest); rec->subs = selabel_subs_init(selinux_file_context_subs_path(), - rec->subs, rec->digest); + rec->digest); path = selinux_file_context_path(); } else { snprintf(subs_file, sizeof(subs_file), "%s.subs_dist", path); - rec->dist_subs = selabel_subs_init(subs_file, rec->dist_subs, - rec->digest); + rec->dist_subs = selabel_subs_init(subs_file, rec->digest); snprintf(subs_file, sizeof(subs_file), "%s.subs", path); - rec->subs = selabel_subs_init(subs_file, rec->subs, - rec->digest); + rec->subs = selabel_subs_init(subs_file, rec->digest); } #endif diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h index 3d7ff748..b03652eb 100644 --- a/libselinux/src/label_internal.h +++ b/libselinux/src/label_internal.h @@ -76,7 +76,6 @@ extern int digest_add_specfile(struct selabel_digest *digest, FILE *fp, extern void digest_gen_hash(struct selabel_digest *digest); extern struct selabel_sub *selabel_subs_init(const char *path, - struct selabel_sub *list, struct selabel_digest *digest); struct selabel_lookup_rec {