[PATCH] libsemanage: get_home_dirs: cleanup parsing of values from conf files

[Rahul Sandhu <nvraxn@gmail.com>]

atoi (3) is... bugprone.  It's virtually impossible to differentiate an
invalid value (e.g. the string "foo") from a valid value such as "0" as
0 is returned on error!  From the manual page:

>       except that atoi() does not detect errors.
> RETURN VALUE
>       The converted value or 0 on error.

In the case of get_home_dirs, atoi is downright wrong.  We are parsing
UID_MIN, UID_MAX, and LU_UIDNUMBER, which all have a numerical value,
without any validation that what we are parsing is actually a number.
This is especially problematic as that means that in the case of an
invalid value (e.g. UID_MIN=foo), UID_MIN is incorrectly parsed as 0.

Instead, use strtoul (3) to parse these values.  If parsing fails, such
as in the case where UID_MIN=foo, warn that parsing failed, and use the
default values for each key as specified by the manual page.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 libsemanage/src/genhomedircon.c | 41 ++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 8782e2cb..a7b44d8d 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -354,24 +354,53 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 
 	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL);
 	if (path && *path) {
-		temp = atoi(path);
-		minuid = temp;
-		minuid_set = 1;
+		char *endptr;
+		const unsigned long val = strtoul(path, &endptr, 0);
+		if (endptr != path && *endptr == '\0') {
+			minuid = (uid_t)val;
+			minuid_set = 1;
+		} else {
+			/* we were provided an invalid value, use defaults.  */
+			WARN(s->h_semanage,
+			     "Conversion failed for key UID_MIN, is its value a number?"
+			     "  Falling back to default value of `1000`.");
+			minuid = 1000;
+			minuid_set = 1;
+		}
 	}
 	free(path);
 	path = NULL;
 
 	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
 	if (path && *path) {
-		temp = atoi(path);
-		maxuid = temp;
+		char *endptr;
+		const unsigned long val = strtoul(path, &endptr, 0);
+		if (endptr != path && *endptr == '\0') {
+			maxuid = (uid_t)val;
+		} else {
+			/* we were provided an invalid value, use defaults.  */
+			WARN(s->h_semanage,
+			     "Conversion failed for key UID_MAX, is its value a number?"
+			     "  Falling back to default value of `6000`.");
+			maxuid = 60000;
+		}
 	}
 	free(path);
 	path = NULL;
 
 	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
 	if (path && *path) {
-		temp = atoi(path);
+		char *endptr;
+		const unsigned long val = strtoul(path, &endptr, 0);
+		if (endptr != path && *endptr == '\0') {
+			temp = (uid_t)val;
+		} else {
+			/* we were provided an invalid value, use defaults.  */
+			WARN(s->h_semanage,
+			     "Conversion failed for key LU_UIDNUMBER, is its value a number?"
+			     "  Falling back to default value of `500`.");
+			temp = 500;
+		}
 		if (!minuid_set || temp < minuid) {
 			minuid = temp;
 			minuid_set = 1;
-- 
2.50.1