From cfecac2e71bca14f9207e5879b7c04f965776737 Mon Sep 17 00:00:00 2001 From: Nickolai Zeldovich Date: Tue, 12 Feb 2013 15:08:38 -0500 Subject: [PATCH] Fix scanf buffer overflows Fix potential buffer overflows caused by misuse of the scanf function in the fileserver and ptserver. Also fix similar issues in the client side fs command and libadmin library. Change-Id: Ia6a46981c50537da1673507c2bc777f96e43f95a (This change was applied to the 1.6 branch as a security fix for 1.6.2 as commit d1855f8e04; this commit brings the fix into master.) Reviewed-on: http://gerrit.openafs.org/9962 Reviewed-by: Derrick Brashear Tested-by: BuildBot Reviewed-by: Jeffrey Altman --- src/libacl/aclprocs.c | 8 ++++---- src/libadmin/client/afs_clientAdmin.c | 4 ++-- src/ptserver/ptclient.c | 2 +- src/ptserver/ptprocs.c | 2 +- src/ptserver/readgroup.c | 6 +++--- src/venus/fs.c | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libacl/aclprocs.c b/src/libacl/aclprocs.c index 8643cae6b..d963c78ae 100644 --- a/src/libacl/aclprocs.c +++ b/src/libacl/aclprocs.c @@ -19,13 +19,13 @@ #include #include +#include #include #include #include #include #include "acl.h" - #ifdef AFS_PTHREAD_ENV #include pthread_mutex_t acl_list_mutex; @@ -242,7 +242,7 @@ acl_Internalize_pr(int (*func)(namelist *names, idlist *ids), char *elist, struc if (sscanf(elist, "%d\n%d\n", &p, &n) != 2) return -1; - if (p + n > ACL_MAXENTRIES) + if (p < 0 || n < 0 || p > INT_MAX - n || p + n > ACL_MAXENTRIES) return (-1); acl_NewACL(p + n, acl); (*acl)->total = p + n; @@ -266,7 +266,7 @@ acl_Internalize_pr(int (*func)(namelist *names, idlist *ids), char *elist, struc nextc++; /* now at the beginning of the entry list */ for (i = 0; i < (*acl)->positive; i++) { int k; - if (sscanf(nextc, "%s\t%d\n", lnames.namelist_val[i], &k) != 2) { + if (sscanf(nextc, "%63s\t%d\n", lnames.namelist_val[i], &k) != 2) { free(lnames.namelist_val); return (-1); } @@ -278,7 +278,7 @@ acl_Internalize_pr(int (*func)(namelist *names, idlist *ids), char *elist, struc for (i = (*acl)->total - 1; i >= (*acl)->total - (*acl)->negative; i--, j++) { if (sscanf - (nextc, "%s\t%d\n", lnames.namelist_val[j], + (nextc, "%63s\t%d\n", lnames.namelist_val[j], &((*acl)->entries[j].rights)) != 2) { free(lnames.namelist_val); return (-1); diff --git a/src/libadmin/client/afs_clientAdmin.c b/src/libadmin/client/afs_clientAdmin.c index 6b8967687..d0f61f106 100644 --- a/src/libadmin/client/afs_clientAdmin.c +++ b/src/libadmin/client/afs_clientAdmin.c @@ -1532,7 +1532,7 @@ afsclient_ACLEntryAdd(const char *directory, const char *user, */ is_dfs = - sscanf(old_acl_string, "%d dfs:%d %s", &cur_acl.nplus, &cur_acl.dfs, + sscanf(old_acl_string, "%d dfs:%d %1024s", &cur_acl.nplus, &cur_acl.dfs, cur_acl.cell); ptr = strchr(old_acl_string, '\n'); ptr++; @@ -1557,7 +1557,7 @@ afsclient_ACLEntryAdd(const char *directory, const char *user, */ for (i = 0; i < (cur_acl.nplus + cur_acl.nminus); i++) { - sscanf(ptr, "%s%d\n", cur_user, &cur_user_acl); + sscanf(ptr, "%63s%d\n", cur_user, &cur_user_acl); /* * Skip the entry for the user we are replacing/adding */ diff --git a/src/ptserver/ptclient.c b/src/ptserver/ptclient.c index 019d29b0f..416e4c707 100644 --- a/src/ptserver/ptclient.c +++ b/src/ptserver/ptclient.c @@ -359,7 +359,7 @@ main(int argc, char **argv) foo = line; skip(&foo); for (i = 0; ((lnames.namelist_len < PR_MAXLIST) - && (sscanf(foo, "%s", lnames.namelist_val[i]) != + && (sscanf(foo, "%63s", lnames.namelist_val[i]) != EOF)); i++) { lnames.namelist_len++; skip(&foo); diff --git a/src/ptserver/ptprocs.c b/src/ptserver/ptprocs.c index 9fa7ea624..d97f3347f 100644 --- a/src/ptserver/ptprocs.c +++ b/src/ptserver/ptprocs.c @@ -629,7 +629,7 @@ idToName(struct rx_call *call, idlist *aid, namelist *aname) size = aid->idlist_len; if (size == 0) return 0; - if (size < 0) + if (size < 0 || size > INT_MAX / PR_MAXNAMELEN) return PRTOOMANY; aname->namelist_val = malloc(size * PR_MAXNAMELEN); aname->namelist_len = 0; diff --git a/src/ptserver/readgroup.c b/src/ptserver/readgroup.c index d4a7aaf89..92cd4705d 100644 --- a/src/ptserver/readgroup.c +++ b/src/ptserver/readgroup.c @@ -122,7 +122,7 @@ main(int argc, char **argv) /* grab the group name */ memset(gname, 0, PR_MAXNAMELEN); memset(owner, 0, PR_MAXNAMELEN); - sscanf(buf, "%s %d", gname, &id); + sscanf(buf, "%63s %d", gname, &id); tmp = buf; skip(&tmp); skip(&tmp); @@ -149,7 +149,7 @@ main(int argc, char **argv) if (!fail) { /* read members out of buf and add to the group */ memset(name, 0, PR_MAXNAMELEN); - while (sscanf(tmp, "%s", name) != EOF) { + while (sscanf(tmp, "%63s", name) != EOF) { if (strchr(name, ':') == NULL) { /* then it's not a group */ code = pr_AddToGroup(name, gname); @@ -187,7 +187,7 @@ main(int argc, char **argv) memset(name, 0, PR_MAXNAMELEN); tmp = buf; tmp++; - while (sscanf(tmp, "%s", name) != EOF) { + while (sscanf(tmp, "%63s", name) != EOF) { if (strchr(name, ':') == NULL) { /* then it's not a group */ code = pr_AddToGroup(name, gname); diff --git a/src/venus/fs.c b/src/venus/fs.c index 4ba8ebbde..cd2f82735 100644 --- a/src/venus/fs.c +++ b/src/venus/fs.c @@ -577,7 +577,7 @@ EmptyAcl(char *astr) tp->nplus = tp->nminus = 0; tp->pluslist = tp->minuslist = 0; tp->dfs = 0; - sscanf(astr, "%d dfs:%d %s", &junk, &tp->dfs, tp->cell); + sscanf(astr, "%d dfs:%d %1024s", &junk, &tp->dfs, tp->cell); return tp; } @@ -592,7 +592,7 @@ ParseAcl(char *astr) ta = malloc(sizeof(struct Acl)); assert(ta); ta->dfs = 0; - sscanf(astr, "%d dfs:%d %s", &ta->nplus, &ta->dfs, ta->cell); + sscanf(astr, "%d dfs:%d %1024s", &ta->nplus, &ta->dfs, ta->cell); astr = SkipLine(astr); sscanf(astr, "%d", &ta->nminus); astr = SkipLine(astr); @@ -603,7 +603,7 @@ ParseAcl(char *astr) last = 0; first = 0; for (i = 0; i < nplus; i++) { - sscanf(astr, "%100s %d", tname, &trights); + sscanf(astr, "%99s %d", tname, &trights); astr = SkipLine(astr); tl = malloc(sizeof(struct AclEntry)); assert(tl); @@ -621,7 +621,7 @@ ParseAcl(char *astr) last = 0; first = 0; for (i = 0; i < nminus; i++) { - sscanf(astr, "%100s %d", tname, &trights); + sscanf(astr, "%99s %d", tname, &trights); astr = SkipLine(astr); tl = malloc(sizeof(struct AclEntry)); assert(tl); -- 2.39.5