Fix scanf buffer overflows
authorNickolai Zeldovich <nickolai@csail.mit.edu>
Tue, 12 Feb 2013 20:08:38 +0000 (15:08 -0500)
committerJeffrey Altman <jaltman@your-file-system.com>
Tue, 4 Jun 2013 22:32:42 +0000 (15:32 -0700)
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 <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

src/libacl/aclprocs.c
src/libadmin/client/afs_clientAdmin.c
src/ptserver/ptclient.c
src/ptserver/ptprocs.c
src/ptserver/readgroup.c
src/venus/fs.c

index 8643cae..d963c78 100644 (file)
 #include <roken.h>
 #include <afs/opr.h>
 
+#include <limits.h>
 #include <rx/xdr.h>
 #include <rx/rx.h>
 #include <afs/ptclient.h>
 #include <afs/ptuser.h>
 
 #include "acl.h"
-
 #ifdef AFS_PTHREAD_ENV
 #include <pthread.h>
 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);
index 6b89676..d0f61f1 100644 (file)
@@ -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
         */
index 019d29b..416e4c7 100644 (file)
@@ -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);
index 9fa7ea6..d97f334 100644 (file)
@@ -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;
index d4a7aaf..92cd470 100644 (file)
@@ -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);
index 4ba8ebb..cd2f827 100644 (file)
@@ -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);