From abaa4c9620842e437838597e7123f465808e2bf4 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 --- 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 106dfe356..72bf8aebc 100644 --- a/src/libacl/aclprocs.c +++ b/src/libacl/aclprocs.c @@ -23,13 +23,13 @@ #else #include #endif +#include #include #include #include #include #include #include "acl.h" - #ifdef AFS_PTHREAD_ENV #include #include @@ -251,7 +251,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; @@ -276,7 +276,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); } @@ -288,7 +288,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 3f50786a7..35dbd6c90 100644 --- a/src/libadmin/client/afs_clientAdmin.c +++ b/src/libadmin/client/afs_clientAdmin.c @@ -1542,7 +1542,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++; @@ -1567,7 +1567,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 bb47339d1..d4d57c0e5 100644 --- a/src/ptserver/ptclient.c +++ b/src/ptserver/ptclient.c @@ -367,7 +367,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 ee56bcf3a..ae1a56292 100644 --- a/src/ptserver/ptprocs.c +++ b/src/ptserver/ptprocs.c @@ -679,7 +679,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 = (prname *) malloc(size * PR_MAXNAMELEN); aname->namelist_len = 0; diff --git a/src/ptserver/readgroup.c b/src/ptserver/readgroup.c index 958378d00..2404fc91d 100644 --- a/src/ptserver/readgroup.c +++ b/src/ptserver/readgroup.c @@ -111,7 +111,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); @@ -138,7 +138,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); @@ -176,7 +176,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 5ffa27b5d..02f7cd5fe 100644 --- a/src/venus/fs.c +++ b/src/venus/fs.c @@ -561,7 +561,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; } @@ -576,7 +576,7 @@ ParseAcl(char *astr) ta = (struct Acl *)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); @@ -587,7 +587,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 = (struct AclEntry *)malloc(sizeof(struct AclEntry)); assert(tl); @@ -605,7 +605,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 = (struct AclEntry *)malloc(sizeof(struct AclEntry)); assert(tl); -- 2.39.5