coverity: handle strto[*] functions under|over-flows robustly

a recent change that we (I) made to things wasn't handling under|over-flows properly. The man page for strtol states: Since strtol() can legitimately return 0, LONG_MAX, or LONG_MIN on both success and failure, the calling program should set errno to 0 before the call, and then determine if an error occurred by checking whether errno has a nonzero value after the call. Since all strto* functions (strtoll, strtol, strtoul, strtod, strtof) are similar - set errno to 0, and see if it is errno is set to ERANGE when it's finished. checked with 'grep -A 1 -B 1 strto * -R' in the repo Review the entire code base, and do the same thing everywhere. tested on x86 and Zynq + FMCOMMS3 Signed-off-by: Robin Getz <[email protected]>
analogdevicesinc · May 22, 2020 · 3267944 · 3267944
1 parent ab9a0e7
commit 3267944
Show file tree

Hide file tree

Showing 13 changed files with 108 additions and 37 deletions.
diff --git a/channel.c b/channel.c
@@ -710,8 +710,9 @@ int iio_channel_attr_read_longlong(const struct iio_channel *chn,
 	if (ret < 0)
 		return (int) ret;
 
+	errno = 0;
 	value = strtoll(buf, &end, 0);
-	if (end == buf)
+	if (end == buf || errno == ERANGE)
 		return -EINVAL;
 	*val = value;
 	return 0;

diff --git a/device.c b/device.c
@@ -622,8 +622,9 @@ int iio_device_attr_read_longlong(const struct iio_device *dev,
 	if (ret < 0)
 		return (int) ret;
 
+	errno = 0;
 	value = strtoll(buf, &end, 0);
-	if (end == buf)
+	if (end == buf || errno == ERANGE)
 		return -EINVAL;
 	*val = value;
 	return 0;
@@ -698,8 +699,9 @@ int iio_device_buffer_attr_read_longlong(const struct iio_device *dev,
 	if (ret < 0)
 		return (int) ret;
 
+	errno = 0;
 	value = strtoll(buf, &end, 0);
-	if (end == buf)
+	if (end == buf || errno == ERANGE)
 		return -EINVAL;
 	*val = value;
 	return 0;
@@ -814,8 +816,9 @@ int iio_device_debug_attr_read_longlong(const struct iio_device *dev,
 	if (ret < 0)
 		return (int) ret;
 
+	errno = 0;
 	value = strtoll(buf, &end, 0);
-	if (end == buf)
+	if (end == buf || errno == ERANGE)
 		return -EINVAL;
 	*val = value;
 	return 0;

diff --git a/examples/iio-monitor.c b/examples/iio-monitor.c
@@ -82,7 +82,7 @@ static void err_str(int ret)
 
 static double get_channel_value(struct iio_channel *chn)
 {
-	char *old_locale;
+	char *old_locale, *end;
 	char buf[1024];
 	double val;
 	int ret;
@@ -95,30 +95,49 @@ static double get_channel_value(struct iio_channel *chn)
 		if (ret < 0) {
 			err_str(ret);
 			val = 0;
-		} else
-			val = strtod(buf, NULL);
+		} else {
+			errno = 0;
+			val = strtod(buf, &end);
+			if (buf == end || errno == ERANGE) {
+				fprintf(stderr, "issue decoding '%s' to decimal\n", buf);
+				val = 0;
+			}
+		}
 	} else {
 		ret = iio_channel_attr_read(chn, "raw", buf, sizeof(buf));
 		if (ret < 0) {
 			err_str(ret);
 			val = 0;
-		} else
-			val = strtod(buf, NULL);
-
+		} else {
+			errno = 0;
+			val = strtod(buf, &end);
+			if (buf == end || errno == ERANGE) {
+				fprintf(stderr, "issue decoding '%s' to decimal\n", buf);
+				val = 0;
+			}
+		}
 		if (channel_has_attr(chn, "offset")) {
 			ret = iio_channel_attr_read(chn, "offset", buf, sizeof(buf));
 			if (ret < 0)
 				err_str(ret);
-			else
-				val += strtod(buf, NULL);
+			else {
+				errno = 0;
+				val += strtod(buf, &end);
+				if (buf == end || errno == ERANGE)
+					fprintf(stderr, "issue decoding '%s' to decimal\n", buf);
+			}
 		}
 
 		if (channel_has_attr(chn, "scale")) {
 			ret = iio_channel_attr_read(chn, "scale", buf, sizeof(buf));
 			if (ret < 0)
 				err_str(ret);
-			else
-				val *= strtod(buf, NULL);
+			else {
+				errno = 0;
+				val *= strtod(buf, &end);
+				if (buf == end || errno == ERANGE)
+					fprintf(stderr, "issue decoding '%s' to decimal\n", buf);
+			}
 		}
 	}
 

diff --git a/iiod-client.c b/iiod-client.c
@@ -60,8 +60,9 @@ static ssize_t iiod_client_read_integer(struct iiod_client *client,
 
 	buf[i] = '\0';
 
+	errno = 0;
 	value = (int) strtol(ptr, &end, 10);
-	if (ptr == end)
+	if (ptr == end || errno == ERANGE)
 		return -EINVAL;
 
 	*val = value;
@@ -181,13 +182,15 @@ int iiod_client_get_version(struct iiod_client *client, void *desc,
 	if (ret < 0)
 		return ret;
 
+	errno = 0;
 	maj = strtol(ptr, &end, 10);
-	if (ptr == end)
+	if (ptr == end || errno == ERANGE)
 		return -EIO;
 
 	ptr = end + 1;
+	errno = 0;
 	min = strtol(ptr, &end, 10);
-	if (ptr == end)
+	if (ptr == end || errno == ERANGE)
 		return -EIO;
 
 	ptr = end + 1;

diff --git a/iiod/iiod.c b/iiod/iiod.c
@@ -634,8 +634,9 @@ int main(int argc, char **argv)
 #endif
 		case 'n':
 #ifdef WITH_IIOD_USBD
+			errno = 0;
 			nb_pipes = strtol(optarg, &end, 10);
-			if (optarg == end || nb_pipes < 1) {
+			if (optarg == end || nb_pipes < 1 || errno == ERANGE) {
 				IIO_ERROR("--nb-pipes: Invalid parameter\n");
 				return EXIT_FAILURE;
 			}

diff --git a/iiod/lexer.l b/iiod/lexer.l
@@ -143,7 +143,15 @@ WORD (([[:alpha:]]+,)|(iio:))?(-|_|\.|[[:alnum:]])+
 }
 
 <WANT_VALUE>{WORD} {
-	yylval->value = strtol(yytext, NULL, 10);
+	char *end;
+	char errstr[100];
+
+	errno = 0;
+	yylval->value = strtol(yytext, &end, 10);
+	if (yytext == end || errno == ERANGE) {
+		sprintf(errstr,"lex : bad long constant: %s",(char*)yytext);
+		perror(errstr);
+	}
 	return VALUE;
 }
 

diff --git a/local.c b/local.c
@@ -1225,8 +1225,9 @@ static int handle_protected_scan_element_attr(struct iio_channel *chn,
 			char *end;
 			long long value;
 
+			errno = 0;
 			value = strtoll(buf, &end, 0);
-			if (end == buf || value < 0 || value > LONG_MAX)
+			if (end == buf || value < 0 || errno == ERANGE)
 				return -EINVAL;
 
 			chn->index = (long) value;
@@ -1933,8 +1934,9 @@ static void init_data_scale(struct iio_channel *chn)
 	if (ret < 0)
 		return;
 
+	errno = 0;
 	value = strtof(buf, &end);
-	if (end == buf)
+	if (end == buf || errno == ERANGE)
 		return;
 
 	chn->format.with_scale = true;

diff --git a/network.c b/network.c
@@ -749,8 +749,9 @@ static int read_integer(struct iio_network_io_context *io_ctx, long *val)
 	}
 
 	buf[i] = '\0';
+	errno = 0;
 	ret = (ssize_t) strtol(buf, &ptr, 10);
-	if (ptr == buf)
+	if (ptr == buf || errno == ERANGE)
 		return -EINVAL;
 	*val = (long) ret;
 	return 0;

diff --git a/serial.c b/serial.c
@@ -521,7 +521,11 @@ static int serial_parse_params(const char *params,
 	if (!params || !params[0])
 		return 0;
 
+	errno = 0;
 	*baud_rate = strtoul(params, &end, 10);
+	if (params == end || errno == ERANGE)
+		return -EINVAL;
+
 	/* 110 baud to 1,000,000 baud */
 	if (params == end || *baud_rate < 110 || *baud_rate > 1000001) {
 		IIO_ERROR("Invalid baud rate\n");
@@ -536,8 +540,9 @@ static int serial_parse_params(const char *params,
 
 	params = (const char *)(end);
 
+	errno = 0;
 	*bits = strtoul(params, &end, 10);
-	if (params == end || *bits > 9 || *bits < 5) {
+	if (params == end || *bits > 9 || *bits < 5 || errno == ERANGE) {
 		IIO_ERROR("Invalid number of bits\n");
 		return -EINVAL;
 	}
@@ -571,9 +576,10 @@ static int serial_parse_params(const char *params,
 	params = (const char *)(end);
 	if (!*params)
 		return 0;
-	*stop = strtoul(params, &end, 10);
 
-	if (params == end || !*stop || *stop > 2) {
+	errno = 0;
+	*stop = strtoul(params, &end, 10);
+	if (params == end || !*stop || *stop > 2 || errno == ERANGE) {
 		IIO_ERROR("Invalid number of stop bits\n");
 		return -EINVAL;
 	}

diff --git a/tests/iio_common.c b/tests/iio_common.c
@@ -133,14 +133,17 @@ unsigned long int sanitize_clamp(const char *name, const char *argv,
 	uint64_t min, uint64_t max)
 {
 	uint64_t val;
-	char buf[20];
+	char buf[20], *end;
 
 	if (!argv) {
 		val = 0;
 	} else {
 		/* sanitized buffer by taking first 20 (or less) char */
 		iio_snprintf(buf, sizeof(buf), "%s", argv);
-		val = strtoul(buf, NULL, 10);
+		errno = 0;
+		val = strtoul(buf, &end, 10);
+		if (buf == end || errno == ERANGE)
+			val = 0;
 	}
 
 	if (val > max) {

diff --git a/usb.c b/usb.c
@@ -1131,16 +1131,18 @@ struct iio_context * usb_create_context_from_uri(const char *uri)
 	if (!isdigit(*ptr))
 		goto err_bad_uri;
 
+	errno = 0;
 	bus = strtol(ptr, &end, 10);
-	if (ptr == end || *end != '.')
+	if (ptr == end || *end != '.' || errno == ERANGE)
 		goto err_bad_uri;
 
 	ptr = (const char *) ((uintptr_t) end + 1);
 	if (!isdigit(*ptr))
 		goto err_bad_uri;
 
+	errno = 0;
 	address = strtol(ptr, &end, 10);
-	if (ptr == end)
+	if (ptr == end || errno == ERANGE)
 		goto err_bad_uri;
 
 	if (*end == '\0') {
@@ -1150,8 +1152,9 @@ struct iio_context * usb_create_context_from_uri(const char *uri)
 		if (!isdigit(*ptr))
 			goto err_bad_uri;
 
+		errno = 0;
 		intrfc = strtol(ptr, &end, 10);
-		if (ptr == end || *end != '\0')
+		if (ptr == end || *end != '\0' || errno == ERANGE)
 			goto err_bad_uri;
 	} else {
 		goto err_bad_uri;

diff --git a/utilities.c b/utilities.c
@@ -42,6 +42,7 @@ static int read_double_locale(const char *str, double *val)
 {
 	char *end, *old_locale;
 	double value;
+	bool problem = false;
 
 	/* XXX: This is not thread-safe, but it's the only way we have to
 	 * support locales under MinGW without linking with Visual Studio
@@ -51,11 +52,16 @@ static int read_double_locale(const char *str, double *val)
 		return -ENOMEM;
 
 	setlocale(LC_NUMERIC, "C");
+
+	errno = 0;
 	value = strtod(str, &end);
+	if (end == str || errno == ERANGE)
+		problem = true;
+
 	setlocale(LC_NUMERIC, old_locale);
 	free(old_locale);
 
-	if (end == str)
+	if (problem)
 		return -EINVAL;
 
 	*val = value;
@@ -80,14 +86,20 @@ static int read_double_locale(const char *str, double *val)
 {
 	char *end;
 	double value;
+	bool problem = false;
+
 	_locale_t locale = _create_locale(LC_NUMERIC, "C");
 	if (!locale)
 		return -ENOMEM;
 
+	errno = 0;
 	value = _strtod_l(str, &end, locale);
+	if (end == str || errno == ERANGE)
+		problem = true;
+
 	_free_locale(locale);
 
-	if (end == str)
+	if (problem)
 		return -EINVAL;
 
 	*val = value;
@@ -109,6 +121,7 @@ static int read_double_locale(const char *str, double *val)
 {
 	char *end;
 	double value;
+	bool problem = false;
 	locale_t old_locale, new_locale;
 
 	new_locale = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0);
@@ -117,11 +130,15 @@ static int read_double_locale(const char *str, double *val)
 
 	old_locale = uselocale(new_locale);
 
+	errno = 0;
 	value = strtod(str, &end);
+	if (end == str || errno == ERANGE)
+		problem = true;
+
 	uselocale(old_locale);
 	freelocale(new_locale);
 
-	if (end == str)
+	if (problem)
 		return -EINVAL;
 
 	*val = value;
@@ -153,9 +170,11 @@ int read_double(const char *str, double *val)
 	return read_double_locale(str, val);
 #else
 	char *end;
-	double value = strtod(str, &end);
+	double value;
 
-	if (end == str)
+	errno = 0;
+	value = strtod(str, &end);
+	if (end == str || errno == ERANGE)
 		return -EINVAL;
 
 	*val = value;

diff --git a/xml.c b/xml.c
@@ -142,8 +142,9 @@ static void setup_scan_element(struct iio_channel *chn, xmlNode *n)
 			char *end;
 			long long value;
 
+			errno = 0;
 			value = strtoll(content, &end, 0);
-			if (end == content || value < 0 || value > LONG_MAX)
+			if (end == content || value < 0 || errno == ERANGE)
 				return;
 			chn->index = (long) value;
 		} else if (!strcmp(name, "format")) {
@@ -179,8 +180,9 @@ static void setup_scan_element(struct iio_channel *chn, xmlNode *n)
 			char *end;
 			float value;
 
+			errno = 0;
 			value = strtof(content, &end);
-			if (end == content) {
+			if (end == content || errno == ERANGE) {
 				chn->format.with_scale = false;
 				return;
 			}