From 46ebb6869b87b06ad54bbdaa844a7b4e24b10914 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Sun, 24 Feb 2013 15:05:48 -0800 Subject: [PATCH] OpenAFS-SA-2013-0001: Buffer overflow in OpenAFS fileserver By carefully crafting an ACL entry an attacker may overflow fixed length buffers within the OpenAFS fileserver, crashing the fileserver, and potentially permitting the execution of arbitrary code. To perform the exploit, the attacker must already have permissions to create ACLs on the fileserver in question. Once such an ACL is present on a fileserver, client utilities such as 'fs' which manipulate ACLs, may be crashed when they attempt to read or modify the ACL. --- src/libacl/aclprocs.c | 7 ++++--- src/libadmin/client/afs_clientAdmin.c | 4 ++-- src/venus/fs.c | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/libacl/aclprocs.c b/src/libacl/aclprocs.c index f294d0949..5a1dbdc8c 100644 --- a/src/libacl/aclprocs.c +++ b/src/libacl/aclprocs.c @@ -23,6 +23,7 @@ #else #include #endif +#include #include #include #include @@ -247,7 +248,7 @@ acl_Internalize(elist, acl) 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; @@ -272,7 +273,7 @@ acl_Internalize(elist, acl) 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); } @@ -284,7 +285,7 @@ acl_Internalize(elist, acl) 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 a228d0ac4..ae7b387ee 100644 --- a/src/libadmin/client/afs_clientAdmin.c +++ b/src/libadmin/client/afs_clientAdmin.c @@ -1530,7 +1530,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++; @@ -1555,7 +1555,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/venus/fs.c b/src/venus/fs.c index fb4f4f1db..7b4a992a3 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