Fixed potential buffer overflow when converting strings to GIDs/UIDs

larsewi · larsewi · commit 348b46a1bdac · 2025-12-11T11:53:36.000+01:00
Ticket: ENT-13551 Changelog: Title Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
diff --git a/cf-agent/cf-agent.c b/cf-agent/cf-agent.c
@@ -1766,7 +1766,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;
         }
@@ -1786,7 +1786,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;
diff --git a/libpromises/conversion.c b/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
diff --git a/libpromises/conversion.h b/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
diff --git a/libpromises/evalfunction.c b/libpromises/evalfunction.c
@@ -1542,7 +1542,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();
@@ -1592,7 +1592,7 @@ static FnCallResult FnCallGetGroupInfo(ARG_UNUSED EvalContext *ctx, ARG_UNUSED c
         char *arg = RlistScalarValue(finalargs);
         if (StringIsNumeric(arg))
         {
-            gid_t gid = Str2Gid(arg, NULL, NULL);
+            gid_t gid = Str2Gid(arg, NULL, 0, NULL);
             if (gid == CF_SAME_GROUP) // user "*"
             {
                 gid = getgid();
@@ -9261,7 +9261,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();
@@ -9288,7 +9288,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();
diff --git a/libpromises/policy.c b/libpromises/policy.c
@@ -2964,7 +2964,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)
@@ -2978,7 +2977,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;
@@ -3006,7 +3005,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)
@@ -3020,7 +3018,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;

Original file line number	Diff line number	Diff line change
`@@ -1766,7 +1766,7 @@ static void CheckAgentAccess(const Rlist list, const Policy policy)`
`1766`	`1766`
`1767`	`1767`	`for (const Rlist *rp = list; rp != NULL; rp = rp->next)`
`1768`	`1768`	`{`
`1769`		`- if (Str2Uid(RlistScalarValue(rp), NULL, NULL) == uid)`
	`1769`	`+ if (Str2Uid(RlistScalarValue(rp), NULL, 0, NULL) == uid)`
`1770`	`1770`	`{`
`1771`	`1771`	`return;`
`1772`	`1772`	`}`
`@@ -1786,7 +1786,7 @@ static void CheckAgentAccess(const Rlist list, const Policy policy)`
`1786`	`1786`	`bool access = false;`
`1787`	`1787`	`for (const Rlist *rp2 = ACCESSLIST; rp2 != NULL; rp2 = rp2->next)`
`1788`	`1788`	`{`
`1789`		`- if (Str2Uid(RlistScalarValue(rp2), NULL, NULL) == sb.st_uid)`
	`1789`	`+ if (Str2Uid(RlistScalarValue(rp2), NULL, 0, NULL) == sb.st_uid)`
`1790`	`1790`	`{`
`1791`	`1791`	`access = true;`
`1792`	`1792`	`break;`
Original file line number	Diff line number	Diff line change
`@@ -988,7 +988,7 @@ UidList Rlist2UidList(Rlist uidnames, const Promise *pp)`
`988`	`988`	`for (rp = uidnames; rp != NULL; rp = rp->next)`
`989`	`989`	`{`
`990`	`990`	`username[0] = '\0';`
`991`		`- uid = Str2Uid(RlistScalarValue(rp), username, pp);`
	`991`	`+ uid = Str2Uid(RlistScalarValue(rp), username, sizeof(username), pp);`
`992`	`992`	`AddSimpleUidItem(&uidlist, uid, username);`
`993`	`993`	`}`
`994`	`994`
`@@ -1049,7 +1049,7 @@ GidList Rlist2GidList(Rlist gidnames, const Promise *pp)`
`1049`	`1049`	`for (rp = gidnames; rp != NULL; rp = rp->next)`
`1050`	`1050`	`{`
`1051`	`1051`	`groupname[0] = '\0';`
`1052`		`- gid = Str2Gid(RlistScalarValue(rp), groupname, pp);`
	`1052`	`+ gid = Str2Gid(RlistScalarValue(rp), groupname, sizeof(groupname), pp);`
`1053`	`1053`	`AddSimpleGidItem(&gidlist, gid, groupname);`
`1054`	`1054`	`}`
`1055`	`1055`
`@@ -1063,7 +1063,7 @@ GidList Rlist2GidList(Rlist gidnames, const Promise *pp)`
`1063`	`1063`
`1064`	`1064`	`/*********************************************************************/`
`1065`	`1065`
`1066`		`-uid_t Str2Uid(const char uidbuff, char usercopy, const Promise *pp)`
	`1066`	`+uid_t Str2Uid(const char uidbuff, char usercopy, size_t copy_size, const Promise *pp)`
`1067`	`1067`	`{`
`1068`	`1068`	`if (StringEqual(uidbuff, "*"))`
`1069`	`1069`	`{`
`@@ -1126,7 +1126,7 @@ uid_t Str2Uid(const char uidbuff, char usercopy, const Promise *pp)`
`1126`	`1126`	`{`
`1127`	`1127`	`if (usercopy != NULL)`
`1128`	`1128`	`{`
`1129`		`- strcpy(usercopy, uidbuff);`
	`1129`	`+ strlcpy(usercopy, uidbuff, copy_size);`
`1130`	`1130`	`}`
`1131`	`1131`	`}`
`1132`	`1132`	`else`
`@@ -1142,7 +1142,7 @@ uid_t Str2Uid(const char uidbuff, char usercopy, const Promise *pp)`
`1142`	`1142`
`1143`	`1143`	`/*********************************************************************/`
`1144`	`1144`
`1145`		`-gid_t Str2Gid(const char gidbuff, char groupcopy, const Promise *pp)`
	`1145`	`+gid_t Str2Gid(const char gidbuff, char groupcopy, size_t copy_size, const Promise *pp)`
`1146`	`1146`	`{`
`1147`	`1147`	`if (StringEqual(gidbuff, "*"))`
`1148`	`1148`	`{`
`@@ -1169,7 +1169,7 @@ gid_t Str2Gid(const char gidbuff, char groupcopy, const Promise *pp)`
`1169`	`1169`	`{`
`1170`	`1170`	`if (groupcopy != NULL)`
`1171`	`1171`	`{`
`1172`		`- strcpy(groupcopy, gidbuff);`
	`1172`	`+ strlcpy(groupcopy, gidbuff, copy_size);`
`1173`	`1173`	`}`
`1174`	`1174`	`}`
`1175`	`1175`	`else`
Original file line number	Diff line number	Diff line change
`@@ -1542,7 +1542,7 @@ static FnCallResult FnCallGetUserInfo(ARG_UNUSED EvalContext *ctx, ARG_UNUSED co`
`1542`	`1542`	`char *arg = RlistScalarValue(finalargs);`
`1543`	`1543`	`if (StringIsNumeric(arg))`
`1544`	`1544`	`{`
`1545`		`- uid_t uid = Str2Uid(arg, NULL, NULL);`
	`1545`	`+ uid_t uid = Str2Uid(arg, NULL, 0, NULL);`
`1546`	`1546`	`if (uid == CF_SAME_OWNER) // user "*"`
`1547`	`1547`	`{`
`1548`	`1548`	`uid = getuid();`
`@@ -1592,7 +1592,7 @@ static FnCallResult FnCallGetGroupInfo(ARG_UNUSED EvalContext *ctx, ARG_UNUSED c`
`1592`	`1592`	`char *arg = RlistScalarValue(finalargs);`
`1593`	`1593`	`if (StringIsNumeric(arg))`
`1594`	`1594`	`{`
`1595`		`- gid_t gid = Str2Gid(arg, NULL, NULL);`
	`1595`	`+ gid_t gid = Str2Gid(arg, NULL, 0, NULL);`
`1596`	`1596`	`if (gid == CF_SAME_GROUP) // user "*"`
`1597`	`1597`	`{`
`1598`	`1598`	`gid = getgid();`
`@@ -9261,7 +9261,7 @@ FnCallResult FnCallUserExists(ARG_UNUSED EvalContext *ctx, ARG_UNUSED const Poli`
`9261`	`9261`
`9262`	`9262`	`if (StringIsNumeric(arg))`
`9263`	`9263`	`{`
`9264`		`- uid_t uid = Str2Uid(arg, NULL, NULL);`
	`9264`	`+ uid_t uid = Str2Uid(arg, NULL, 0, NULL);`
`9265`	`9265`	`if (uid == CF_SAME_OWNER \|\| uid == CF_UNKNOWN_OWNER)`
`9266`	`9266`	`{`
`9267`	`9267`	`return FnFailure();`
`@@ -9288,7 +9288,7 @@ FnCallResult FnCallGroupExists(ARG_UNUSED EvalContext *ctx, ARG_UNUSED const Pol`
`9288`	`9288`
`9289`	`9289`	`if (StringIsNumeric(arg))`
`9290`	`9290`	`{`
`9291`		`- gid_t gid = Str2Gid(arg, NULL, NULL);`
	`9291`	`+ gid_t gid = Str2Gid(arg, NULL, 0, NULL);`
`9292`	`9292`	`if (gid == CF_SAME_GROUP \|\| gid == CF_UNKNOWN_GROUP)`
`9293`	`9293`	`{`
`9294`	`9294`	`return FnFailure();`
Original file line number	Diff line number	Diff line change
`@@ -2964,7 +2964,6 @@ uid_t PromiseGetConstraintAsUid(const EvalContext ctx, const char lval, const`
`2964`	`2964`	`uid_t PromiseGetConstraintAsUid(const EvalContext ctx, const char lval, const Promise *pp)`
`2965`	`2965`	`{`
`2966`	`2966`	`int retval = CF_SAME_OWNER;`
`2967`		`- char buffer[CF_MAXVARSIZE];`
`2968`	`2967`
`2969`	`2968`	`const Constraint *cp = PromiseGetConstraint(pp, lval);`
`2970`	`2969`	`if (cp)`
`@@ -2978,7 +2977,7 @@ uid_t PromiseGetConstraintAsUid(const EvalContext ctx, const char lval, const`
`2978`	`2977`	`FatalError(ctx, "Aborted");`
`2979`	`2978`	`}`
`2980`	`2979`
`2981`		`- retval = Str2Uid((char *) cp->rval.item, buffer, pp);`
	`2980`	`+ retval = Str2Uid((char *) cp->rval.item, NULL, 0, pp);`
`2982`	`2981`	`}`
`2983`	`2982`
`2984`	`2983`	`return retval;`
`@@ -3006,7 +3005,6 @@ gid_t PromiseGetConstraintAsGid(const EvalContext ctx, char lval, const Promis`
`3006`	`3005`	`gid_t PromiseGetConstraintAsGid(const EvalContext ctx, char lval, const Promise *pp)`
`3007`	`3006`	`{`
`3008`	`3007`	`int retval = CF_SAME_GROUP;`
`3009`		`- char buffer[CF_MAXVARSIZE];`
`3010`	`3008`
`3011`	`3009`	`const Constraint *cp = PromiseGetConstraint(pp, lval);`
`3012`	`3010`	`if (cp)`
`@@ -3020,7 +3018,7 @@ gid_t PromiseGetConstraintAsGid(const EvalContext ctx, char lval, const Promis`
`3020`	`3018`	`FatalError(ctx, "Aborted");`
`3021`	`3019`	`}`
`3022`	`3020`
`3023`		`- retval = Str2Gid((char *) cp->rval.item, buffer, pp);`
	`3021`	`+ retval = Str2Gid((char *) cp->rval.item, NULL, 0, pp);`
`3024`	`3022`	`}`
`3025`	`3023`
`3026`	`3024`	`return retval;`