Merge pull request #5998 from larsewi/str2id-3.24.x

ENT-13551: Fixed potential buffer overflow when converting strings to GIDs/UIDs (3.24.x)

Author: larsewi

cf-agent/cf-agent.c

@@ -1667,7 +1667,7 @@ static void CheckAgentAccess(const Rlist *list, const Policy *policy)
 
     for (const Rlist *rp = list; rp != NULL; rp = rp->next)
     {
-        if (Str2Uid(RlistScalarValue(rp), NULL, NULL) == uid)
+        if (Str2Uid(RlistScalarValue(rp), NULL, 0, NULL) == uid)
         {
             return;
         }
@@ -1687,7 +1687,7 @@ static void CheckAgentAccess(const Rlist *list, const Policy *policy)
                 bool access = false;
                 for (const Rlist *rp2 = ACCESSLIST; rp2 != NULL; rp2 = rp2->next)
                 {
-                    if (Str2Uid(RlistScalarValue(rp2), NULL, NULL) == sb.st_uid)
+                    if (Str2Uid(RlistScalarValue(rp2), NULL, 0, NULL) == sb.st_uid)
                     {
                         access = true;
                         break;

libpromises/conversion.c

@@ -988,7 +988,7 @@ UidList *Rlist2UidList(Rlist *uidnames, const Promise *pp)
     for (rp = uidnames; rp != NULL; rp = rp->next)
     {
         username[0] = '\0';
-        uid = Str2Uid(RlistScalarValue(rp), username, pp);
+        uid = Str2Uid(RlistScalarValue(rp), username, sizeof(username), pp);
         AddSimpleUidItem(&uidlist, uid, username);
     }
 
@@ -1049,7 +1049,7 @@ GidList *Rlist2GidList(Rlist *gidnames, const Promise *pp)
     for (rp = gidnames; rp != NULL; rp = rp->next)
     {
         groupname[0] = '\0';
-        gid = Str2Gid(RlistScalarValue(rp), groupname, pp);
+        gid = Str2Gid(RlistScalarValue(rp), groupname, sizeof(groupname), pp);
         AddSimpleGidItem(&gidlist, gid, groupname);
     }
 
@@ -1063,7 +1063,7 @@ GidList *Rlist2GidList(Rlist *gidnames, const Promise *pp)
 
 /*********************************************************************/
 
-uid_t Str2Uid(const char *uidbuff, char *usercopy, const Promise *pp)
+uid_t Str2Uid(const char *uidbuff, char *usercopy, size_t copy_size, const Promise *pp)
 {
     if (StringEqual(uidbuff, "*"))
     {
@@ -1126,7 +1126,7 @@ uid_t Str2Uid(const char *uidbuff, char *usercopy, const Promise *pp)
     {
         if (usercopy != NULL)
         {
-            strcpy(usercopy, uidbuff);
+            strlcpy(usercopy, uidbuff, copy_size);
         }
     }
     else
@@ -1142,7 +1142,7 @@ uid_t Str2Uid(const char *uidbuff, char *usercopy, const Promise *pp)
 
 /*********************************************************************/
 
-gid_t Str2Gid(const char *gidbuff, char *groupcopy, const Promise *pp)
+gid_t Str2Gid(const char *gidbuff, char *groupcopy, size_t copy_size, const Promise *pp)
 {
     if (StringEqual(gidbuff, "*"))
     {
@@ -1169,7 +1169,7 @@ gid_t Str2Gid(const char *gidbuff, char *groupcopy, const Promise *pp)
     {
         if (groupcopy != NULL)
         {
-            strcpy(groupcopy, gidbuff);
+            strlcpy(groupcopy, gidbuff, copy_size);
         }
     }
     else

libpromises/conversion.h

@@ -83,8 +83,8 @@ void GidListDestroy(GidList *gids);
 UidList *Rlist2UidList(Rlist *uidnames, const Promise *pp);
 GidList *Rlist2GidList(Rlist *gidnames, const Promise *pp);
 #ifndef __MINGW32__
-uid_t Str2Uid(const char *uidbuff, char *copy, const Promise *pp);
-gid_t Str2Gid(const char *gidbuff, char *copy, const Promise *pp);
+uid_t Str2Uid(const char *uidbuff, char *copy, size_t copy_size, const Promise *pp);
+gid_t Str2Gid(const char *gidbuff, char *copy, size_t copy_size, const Promise *pp);
 #endif /* !__MINGW32__ */
 
 #endif

libpromises/evalfunction.c

@@ -1121,7 +1121,7 @@ static FnCallResult FnCallGetUserInfo(ARG_UNUSED EvalContext *ctx, ARG_UNUSED co
         char *arg = RlistScalarValue(finalargs);
         if (StringIsNumeric(arg))
         {
-            uid_t uid = Str2Uid(arg, NULL, NULL);
+            uid_t uid = Str2Uid(arg, NULL, 0, NULL);
             if (uid == CF_SAME_OWNER) // user "*"
             {
                 uid = getuid();
@@ -8129,7 +8129,7 @@ FnCallResult FnCallUserExists(ARG_UNUSED EvalContext *ctx, ARG_UNUSED const Poli
 
     if (StringIsNumeric(arg))
     {
-        uid_t uid = Str2Uid(arg, NULL, NULL);
+        uid_t uid = Str2Uid(arg, NULL, 0, NULL);
         if (uid == CF_SAME_OWNER || uid == CF_UNKNOWN_OWNER)
         {
             return FnFailure();
@@ -8156,7 +8156,7 @@ FnCallResult FnCallGroupExists(ARG_UNUSED EvalContext *ctx, ARG_UNUSED const Pol
 
     if (StringIsNumeric(arg))
     {
-        gid_t gid = Str2Gid(arg, NULL, NULL);
+        gid_t gid = Str2Gid(arg, NULL, 0, NULL);
         if (gid == CF_SAME_GROUP || gid == CF_UNKNOWN_GROUP)
         {
             return FnFailure();

libpromises/policy.c

@@ -2931,7 +2931,6 @@ uid_t PromiseGetConstraintAsUid(const EvalContext *ctx, const char *lval, const
 uid_t PromiseGetConstraintAsUid(const EvalContext *ctx, const char *lval, const Promise *pp)
 {
     int retval = CF_SAME_OWNER;
-    char buffer[CF_MAXVARSIZE];
 
     const Constraint *cp = PromiseGetConstraint(pp, lval);
     if (cp)
@@ -2945,7 +2944,7 @@ uid_t PromiseGetConstraintAsUid(const EvalContext *ctx, const char *lval, const
             FatalError(ctx, "Aborted");
         }
 
-        retval = Str2Uid((char *) cp->rval.item, buffer, pp);
+        retval = Str2Uid((char *) cp->rval.item, NULL, 0, pp);
     }
 
     return retval;
@@ -2973,7 +2972,6 @@ gid_t PromiseGetConstraintAsGid(const EvalContext *ctx, char *lval, const Promis
 gid_t PromiseGetConstraintAsGid(const EvalContext *ctx, char *lval, const Promise *pp)
 {
     int retval = CF_SAME_GROUP;
-    char buffer[CF_MAXVARSIZE];
 
     const Constraint *cp = PromiseGetConstraint(pp, lval);
     if (cp)
@@ -2987,7 +2985,7 @@ gid_t PromiseGetConstraintAsGid(const EvalContext *ctx, char *lval, const Promis
             FatalError(ctx, "Aborted");
         }
 
-        retval = Str2Gid((char *) cp->rval.item, buffer, pp);
+        retval = Str2Gid((char *) cp->rval.item, NULL, 0, pp);
     }
 
     return retval;