OPENAFS-SA-2024-002: Avoid uninitialized memory when parsing ACLs

CVE-2024-10396

Several places in the tree parse ACLs using sscanf() calls that look
similar to this:

    sscanf(str, "%d dfs:%d %s", &nplus, &dfs, cell);
    sscanf(str, "%100s %d", tname, &trights);

Some callers check whether the scanf() returns negative or 0, but some
callers do not check the return code at all. If only some of the fields
are present in the sscanf()'d string (because, for instance, the ACL is
malformed), some of the arguments are left alone, and may be set to
garbage if the relevant variable was never initialized.

If the parsed ACL is copied to another ACL, this can result in the
copied ACL containing uninitialized memory.

To avoid this, make sure all of the variables passed to sscanf() and
similar calls are initialized before parsing. This commit does not
guarantee that the results make sense, but at least the results do not
contain uninitialized memory.

Change-Id: Ib0becffbce5a6e15f91ac4390bb9c422d478bea6
Reviewed-on: https://gerrit.openafs.org/15917
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2024-11-05 23:40:24 -06:00 committed by Benjamin Kaduk
parent a4ecb05054
commit ac602a0a56
7 changed files with 29 additions and 26 deletions

View File

@ -167,7 +167,7 @@ EmptyAcl(char *astr)
struct Acl *tp;
int junk;
tp = (struct Acl *)malloc(sizeof (struct Acl));
tp = (struct Acl *)calloc(sizeof(*tp), 1);
assert(tp);
tp->nplus = tp->nminus = 0;
tp->pluslist = tp->minuslist = 0;
@ -232,9 +232,9 @@ CleanAcl(struct Acl *aa, char *cellname)
struct Acl *
ParseAcl (char *astr, int astr_size)
{
int nplus, nminus, i, trights, ret;
int nplus, nminus, i, trights = 0, ret;
size_t len;
char tname[ACL_MAXNAME];
char tname[ACL_MAXNAME + 1] = "";
struct AclEntry *first, *next, *last, *tl;
struct Acl *ta;
@ -281,7 +281,7 @@ ParseAcl (char *astr, int astr_size)
if (ret <= 0)
goto nplus_err;
astr = SkipLine(astr);
tl = (struct AclEntry *) malloc(sizeof (struct AclEntry));
tl = (struct AclEntry *) calloc(sizeof(*tl), 1);
if (tl == NULL)
goto nplus_err;
if (!first)
@ -309,7 +309,7 @@ ParseAcl (char *astr, int astr_size)
if (ret <= 0)
goto nminus_err;
astr = SkipLine(astr);
tl = (struct AclEntry *) malloc(sizeof (struct AclEntry));
tl = (struct AclEntry *) calloc(sizeof(*tl), 1);
if (tl == NULL)
goto nminus_err;
if (!first)

View File

@ -228,7 +228,7 @@ struct AclEntry {
static struct Acl *
ParseAcl(char *astr)
{
int nplus, nminus, i, trights;
int nplus = 0, nminus = 0, i, trights = 0;
char tname[MAXNAME + 1] = "";
struct AclEntry *first, *last, *tl;
struct Acl *ta;
@ -237,7 +237,7 @@ ParseAcl(char *astr)
sscanf(astr, "%d", &nminus);
SkipLine(astr);
ta = malloc(sizeof(struct Acl));
ta = calloc(sizeof(*ta), 1);
ta->nplus = nplus;
last = 0;

View File

@ -396,8 +396,8 @@ struct Acl *
ParseAcl(astr)
char *astr;
{
int nplus, nminus, i, trights;
char tname[MAXNAME];
int nplus = 0, nminus = 0, i, trights = 0;
char tname[MAXNAME + 1] = "";
struct AclEntry *first, *last, *tl;
struct Acl *ta;
sscanf(astr, "%d", &nplus);
@ -405,7 +405,7 @@ ParseAcl(astr)
sscanf(astr, "%d", &nminus);
astr = SkipLine(astr);
ta = malloc(sizeof(struct Acl));
ta = calloc(sizeof(*ta), 1);
ta->nplus = nplus;
ta->nminus = nminus;
@ -414,7 +414,7 @@ ParseAcl(astr)
for (i = 0; i < nplus; i++) {
sscanf(astr, "%100s %d", tname, &trights);
astr = SkipLine(astr);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
if (!first)
first = tl;
strcpy(tl->name, tname);
@ -431,7 +431,7 @@ ParseAcl(astr)
for (i = 0; i < nminus; i++) {
sscanf(astr, "%100s %d", tname, &trights);
astr = SkipLine(astr);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
if (!first)
first = tl;
strcpy(tl->name, tname);

View File

@ -1552,6 +1552,9 @@ afsclient_ACLEntryAdd(const char *directory, const char *user,
char tmp[64 + 35];
int is_dfs;
memset(&cur_acl, 0, sizeof(cur_acl));
memset(cur_user, 0, sizeof(cur_user));
if (client_init == 0) {
tst = ADMCLIENTNOINIT;
goto fail_afsclient_ACLEntryAdd;

View File

@ -66,7 +66,7 @@ RSkipLine(char *astr)
struct Acl *
RParseAcl(char *astr)
{
int nplus, nminus, i, trights;
int nplus = 0, nminus = 0, i, trights = 0;
char tname[MAXNAME + 1] = "";
struct AclEntry *first, *last, *tl;
struct Acl *ta;
@ -75,7 +75,7 @@ RParseAcl(char *astr)
sscanf(astr, "%d", &nminus);
astr = RSkipLine(astr);
ta = malloc(sizeof(struct Acl));
ta = calloc(sizeof(*ta), 1);
ta->nplus = nplus;
ta->nminus = nminus;
@ -84,7 +84,7 @@ RParseAcl(char *astr)
for (i = 0; i < nplus; i++) {
sscanf(astr, "%" opr_stringize(MAXNAME) "s %d", tname, &trights);
astr = RSkipLine(astr);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
if (!first)
first = tl;
strcpy(tl->name, tname);
@ -101,7 +101,7 @@ RParseAcl(char *astr)
for (i = 0; i < nminus; i++) {
sscanf(astr, "%" opr_stringize(MAXNAME) "s %d", tname, &trights);
astr = RSkipLine(astr);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
if (!first)
first = tl;
strcpy(tl->name, tname);

View File

@ -405,7 +405,7 @@ static struct Acl *
ParseAcl(char *a_str)
{ /*ParseAcl */
int nplus, nminus, i, trights;
int nplus = 0, nminus = 0, i, trights = 0;
char tname[MAXNAME + 1] = "";
struct AclEntry *first, *last, *tl;
struct Acl *ta;
@ -422,7 +422,7 @@ ParseAcl(char *a_str)
/*
* Allocate and initialize the first entry.
*/
ta = malloc(sizeof(struct Acl));
ta = calloc(sizeof(*ta), 1);
ta->nplus = nplus;
ta->nminus = nminus;
@ -434,7 +434,7 @@ ParseAcl(char *a_str)
for (i = 0; i < nplus; i++) {
sscanf(a_str, "%" opr_stringize(MAXNAME) "s %d", tname, &trights);
a_str = SkipLine(a_str);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
if (!first)
first = tl;
strcpy(tl->name, tname);
@ -454,7 +454,7 @@ ParseAcl(char *a_str)
for (i = 0; i < nminus; i++) {
sscanf(a_str, "%" opr_stringize(MAXNAME) "s %d", tname, &trights);
a_str = SkipLine(a_str);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
if (!first)
first = tl;
strcpy(tl->name, tname);

View File

@ -573,7 +573,7 @@ EmptyAcl(char *astr)
struct Acl *tp;
int junk;
tp = malloc(sizeof(struct Acl));
tp = calloc(sizeof(*tp), 1);
assert(tp);
tp->nplus = tp->nminus = 0;
tp->pluslist = tp->minuslist = 0;
@ -585,12 +585,12 @@ EmptyAcl(char *astr)
static struct Acl *
ParseAcl(char *astr)
{
int nplus, nminus, i, trights;
char tname[MAXNAME];
int nplus = 0, nminus = 0, i, trights = 0;
char tname[MAXNAME + 1] = "";
struct AclEntry *first, *last, *tl;
struct Acl *ta;
ta = malloc(sizeof(struct Acl));
ta = calloc(sizeof(*ta), 1);
assert(ta);
ta->dfs = 0;
sscanf(astr, "%d dfs:%d %1024s", &ta->nplus, &ta->dfs, ta->cell);
@ -606,7 +606,7 @@ ParseAcl(char *astr)
for (i = 0; i < nplus; i++) {
sscanf(astr, "%99s %d", tname, &trights);
astr = SkipLine(astr);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
assert(tl);
if (!first)
first = tl;
@ -624,7 +624,7 @@ ParseAcl(char *astr)
for (i = 0; i < nminus; i++) {
sscanf(astr, "%99s %d", tname, &trights);
astr = SkipLine(astr);
tl = malloc(sizeof(struct AclEntry));
tl = calloc(sizeof(*tl), 1);
assert(tl);
if (!first)
first = tl;