ID: 101 - 'lib/ipmi_sol.c' - possible int *flow

Commit replaces unhandled strtol() calls with appropriate str2*() functions and
wrappers.

Reported-by: Duncan Idaho

ipmitool/include/ipmitool/helper.h

@@ -50,6 +50,14 @@
 #define tboolean   int
 #endif
 
+/* IPMI spec. - UID 0 reserved, 63 maximum UID which can be used */
+#ifndef IPMI_UID_MIN
+# define IPMI_UID_MIN 1
+#endif
+#ifndef IPMI_UID_MAX
+# define IPMI_UID_MAX 63
+#endif
+
 struct ipmi_intf;
 
 struct valstr {
@@ -77,6 +85,7 @@ int str2uchar(const char * str, uint8_t * uchr_ptr);
 
 int is_fru_id(const char *argv_ptr, uint8_t *fru_id_ptr);
 int is_ipmi_channel_num(const char *argv_ptr, uint8_t *channel_ptr);
+int is_ipmi_user_id(const char *argv_ptr, uint8_t *ipmi_uid_ptr);
 
 uint16_t str2val(const char * str, const struct valstr * vs);
 void print_valstr(const struct valstr * vs, const char * title, int loglevel);

ipmitool/lib/helper.c

@@ -720,7 +720,39 @@ is_ipmi_channel_num(const char *argv_ptr, uint8_t *channel_ptr)
 				|| (*channel_ptr >= 0xE && *channel_ptr <= 0xF))) {
 		return 0;
 	}
-	lprintf(LOG_ERR, "Given Channel number '%s' is out of range.", argv_ptr);
+	lprintf(LOG_ERR,
+			"Given Channel number '%s' is either invalid or out of range.",
+			argv_ptr);
 	lprintf(LOG_ERR, "Channel number must be from ranges: <0..7>, <0xE..0xF>");
 	return (-1);
 }
+
+/* is_ipmi_user_id() - wrapper for str-2-uint IPMI UID conversion. Message is
+ * printed on error.
+ *
+ * @argv_ptr: source string to convert from; usually argv
+ * @ipmi_uid_ptr: pointer where to store result
+ *
+ * returns zero on success
+ * returns (-1) on error and message is printed on STDERR
+ */
+int
+is_ipmi_user_id(const char *argv_ptr, uint8_t *ipmi_uid_ptr)
+{
+	if (!argv_ptr || !ipmi_uid_ptr) {
+		lprintf(LOG_ERR,
+				"is_ipmi_user_id(): invalid argument(s).");
+		return (-1);
+	}
+	if ((str2uchar(argv_ptr, ipmi_uid_ptr) == 0)
+			&& *ipmi_uid_ptr >= IPMI_UID_MIN
+			&& *ipmi_uid_ptr <= IPMI_UID_MAX) {
+		return 0;
+	}
+	lprintf(LOG_ERR,
+			"Given User ID '%s' is either invalid or out of range.",
+			argv_ptr);
+	lprintf(LOG_ERR, "User ID is limited to range <%i..%i>.",
+			IPMI_UID_MIN, IPMI_UID_MAX);
+	return (-1);
+}

ipmitool/lib/ipmi_sol.c

@@ -659,21 +659,14 @@ int ipmi_sol_set_param_isvalid_uint8_t( const char *strval,
 					uint8_t maxval,
 					uint8_t *out_value)
 {
-	char *end;
+	if (str2uint(strval, &out_value) != 0 || (*out_value < minval)
-	long val = strtol(strval, &end, base);
+			|| (*out_value > maxval)) {
-
-	if ((val < minval)
-			|| (val > maxval)
-			|| (*end != '\0')) {
 		lprintf(LOG_ERR, "Invalid value %s for parameter %s",
 			strval, name);
 		lprintf(LOG_ERR, "Valid values are %d-%d", minval, maxval);
 		return -1;
 	}
-	else {
+	return 0;
-		*out_value = val;
-		return 0;
-	}
 }
 
 
@@ -1956,10 +1949,11 @@ ipmi_sol_main(struct ipmi_intf * intf, int argc, char ** argv)
 
 		if (argc == 1)
 			channel = 0x0E; /* Ask about the current channel */
-		else if (argc == 2)
+		else if (argc == 2) {
-			channel = (uint8_t)strtol(argv[1], NULL, 0);
+			if (is_ipmi_channel_num(argv[1], &channel) != 0) {
-		else
+				return (-1);
-		{
+			}
+		} else {
 			print_sol_usage();
 			return -1;
 		}
@@ -1987,13 +1981,16 @@ ipmi_sol_main(struct ipmi_intf * intf, int argc, char ** argv)
 			return -1;
 		}
 
-		if (argc >= 3)
+		if (argc >= 3) {
-		{
+			if (is_ipmi_channel_num(argv[2], &channel) != 0) {
-			channel = (uint8_t)strtol(argv[2], NULL, 0);
+				return (-1);
+			}
 		}
 		if (argc == 4)
 		{
-			userid = (uint8_t)strtol(argv[3], NULL, 0);
+			if (is_ipmi_user_id(argv[3], &userid) != 0) {
+				return (-1);
+			}
 		}
 
 		if (!strncmp(argv[1], "enable", 6))
@@ -2033,12 +2030,17 @@ ipmi_sol_main(struct ipmi_intf * intf, int argc, char ** argv)
 		{
 			if (!strncmp(argv[3], "noguard", 7))
 				guard = 0;
-			else
+			else {
-				channel = (uint8_t)strtol(argv[3], NULL, 0);
+				if (is_ipmi_channel_num(argv[3], &channel) != 0) {
+					return (-1);
+				}
+			}
 		}
 		else if (argc == 5)
 		{
-			channel = (uint8_t)strtol(argv[3], NULL, 0);
+			if (is_ipmi_channel_num(argv[3], &channel) != 0) {
+				return (-1);
+			}
 			if (!strncmp(argv[4], "noguard", 7))
 				guard = 0;
 		}
@@ -2117,12 +2119,20 @@ ipmi_sol_main(struct ipmi_intf * intf, int argc, char ** argv)
 		}
 		if (argc != 1) /* at least 2 */
 		{
-			cnt = strtol(argv[1], NULL, 10);
+			if (str2int(argv[1], &cnt) != 0) {
+				lprintf(LOG_ERR, "Given cnt '%s' is invalid.",
+						argv[1]);
+				return (-1);
+			}
 			if(cnt <= 0) cnt = 200;
 		}
 		if (argc >= 3)
 		{
-			interval = strtol(argv[2], NULL, 10);
+			if (str2int(argv[2], &interval) != 0) {
+				lprintf(LOG_ERR, "Given interval '%s' is invalid.",
+						argv[2]);
+				return (-1);
+			}
 			if(interval < 0) interval = 0;
 		}
 		if (argc >= 4) {

ipmitool/lib/ipmi_user.c

@@ -57,9 +57,6 @@ extern int csv_output;
 #define IPMI_PASSWORD_ENABLE_USER   0x01
 #define IPMI_PASSWORD_SET_PASSWORD  0x02
 #define IPMI_PASSWORD_TEST_PASSWORD 0x03
-/* IPMI spec. - UID 0 reserved, 63 maximum UID that can be used */
-#define IPMI_UID_MIN 1
-#define IPMI_UID_MAX 63
 
 /*
  * ipmi_get_user_access
@@ -214,28 +211,6 @@ dump_user_access_csv(
 		       ipmi_privlvl_vals));
 }
 
-/* get_ipmi_user_id - convert str to uint8_t and make sure value is within UID
- * limits
- *
- * @arg: string we are converting from, usually argv[]
- * @user_id: pointer at uint8_t to store converted value.
- * returns: 0 on success, (-1) null args or conv. err/range issue
- */
-int
-get_ipmi_user_id(const char * arg, uint8_t * user_id)
-{
-	if (arg && user_id)
-	{
-		if ((str2uchar(arg, user_id) == 0) && *user_id >= IPMI_UID_MIN
-				&& *user_id <= IPMI_UID_MAX)
-		{
-			return 0;
-		}
-	} /* if (arg && user_id) */
-	lprintf(LOG_ERR, "User ID is limited to range <1..63>.");
-	return (-1);
-} /* get_ipmi_user_id(...) */
-
 static int
 ipmi_print_user_list(
 		     struct ipmi_intf *intf,
@@ -622,8 +597,9 @@ ipmi_user_main(struct ipmi_intf * intf, int argc, char ** argv)
 			char * password = NULL;
 			int password_length = atoi(argv[2]);
 			uint8_t user_id = 0;
-			if (get_ipmi_user_id(argv[1], &user_id))
+			if (is_ipmi_user_id(argv[1], &user_id)) {
 				return (-1);
+			}
 
 			if (argc == 3)
 			{
@@ -675,8 +651,9 @@ ipmi_user_main(struct ipmi_intf * intf, int argc, char ** argv)
 		{
 			char * password = NULL;
 			uint8_t user_id = 0;
-			if (get_ipmi_user_id(argv[2], &user_id))
+			if (is_ipmi_user_id(argv[2], &user_id)) {
 				return (-1);
+			}
 
 			if (argc == 3)
 			{
@@ -751,8 +728,9 @@ ipmi_user_main(struct ipmi_intf * intf, int argc, char ** argv)
 				print_user_usage();
 				return -1;
 			}
-			if (get_ipmi_user_id(argv[2], &user_id))
+			if (is_ipmi_user_id(argv[2], &user_id)) {
 					return (-1);
+			}
 
 			if (strlen(argv[3]) > 16)
 			{
@@ -798,8 +776,9 @@ ipmi_user_main(struct ipmi_intf * intf, int argc, char ** argv)
 		}
 		priv_level = (priv_level & 0x0f);
 
-		if (get_ipmi_user_id(argv[1], &user_id))
+		if (is_ipmi_user_id(argv[1], &user_id)) {
 			return (-1);
+		}
 
 		retval = ipmi_user_set_userpriv(intf,channel,user_id,priv_level);
 	}
@@ -822,8 +801,9 @@ ipmi_user_main(struct ipmi_intf * intf, int argc, char ** argv)
 			return -1;
 		}
 
-		if (get_ipmi_user_id(argv[1], &user_id))
+		if (is_ipmi_user_id(argv[1], &user_id)) {
 			return (-1);
+		}
 
 		operation = (strncmp(argv[0], "disable", 7) == 0) ?
 			IPMI_PASSWORD_DISABLE_USER : IPMI_PASSWORD_ENABLE_USER;