Consolidate some of the stray integer parsers

There are several ad-hoc int/uint parsers scattered around the code,
add a single helper function for that task and replace the multiple
instances. A simple white-box test case is added for the utility
function.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

5 files changed, 212 insertions, 65 deletions

diff --git a/lib/common/src/parse_size.c b/lib/common/src/parse_size.c
index 3e79a19..d240d19 100644
--- a/lib/common/src/parse_size.c
+++ b/lib/common/src/parse_size.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2019 David Oberhollenzer <goliath@infraroot.at>
  */
 #include "common.h"
+#include "util/parse.h"
 
 #include <ctype.h>
 #include <limits.h>
@@ -12,49 +13,45 @@
 int parse_size(const char *what, size_t *out, const char *str,
 	       size_t reference)
 {
-	const char *in = str;
-	size_t acc = 0, x;
+	sqfs_u64 val;
+	size_t diff;
+	int ret;
 
-	if (!isdigit(*in))
+	ret = parse_uint(str, -1, &diff, 0, SIZE_MAX, &val);
+	if (ret == SQFS_ERROR_OVERFLOW)
+		goto fail_ov;
+	if (ret != 0)
 		goto fail_nan;
 
-	while (isdigit(*in)) {
-		x = *(in++) - '0';
+	*out = val;
 
-		if (SZ_MUL_OV(acc, 10, &acc))
-			goto fail_ov;
-
-		if (SZ_ADD_OV(acc, x, &acc))
-			goto fail_ov;
-	}
-
-	switch (*in) {
+	switch (str[diff]) {
 	case 'k':
 	case 'K':
-		if (SZ_MUL_OV(acc, 1024, &acc))
+		if (SZ_MUL_OV(*out, 1024, out))
 			goto fail_ov;
-		++in;
+		++diff;
 		break;
 	case 'm':
 	case 'M':
-		if (SZ_MUL_OV(acc, 1048576, &acc))
+		if (SZ_MUL_OV(*out, 1048576, out))
 			goto fail_ov;
-		++in;
+		++diff;
 		break;
 	case 'g':
 	case 'G':
-		if (SZ_MUL_OV(acc, 1073741824, &acc))
+		if (SZ_MUL_OV(*out, 1073741824, out))
 			goto fail_ov;
-		++in;
+		++diff;
 		break;
 	case '%':
 		if (reference == 0)
 			goto fail_suffix;
 
-		if (SZ_MUL_OV(acc, reference, &acc))
+		if (SZ_MUL_OV(*out, reference, out))
 			goto fail_ov;
 
-		acc /= 100;
+		*out /= 100;
 		break;
 	case '\0':
 		break;
@@ -62,10 +59,9 @@ int parse_size(const char *what, size_t *out, const char *str,
 		goto fail_suffix;
 	}
 
-	if (*in != '\0')
+	if (str[diff] != '\0')
 		goto fail_suffix;
 
-	*out = acc;
 	return 0;
 fail_nan:
 	fprintf(stderr, "%s: '%s' is not a number.\n", what, str);
diff --git a/lib/tar/src/pax_header.c b/lib/tar/src/pax_header.c
index 5ed9c4b..997880e 100644
--- a/lib/tar/src/pax_header.c
+++ b/lib/tar/src/pax_header.c
@@ -8,27 +8,11 @@
 
 #include "internal.h"
 #include "sqfs/xattr.h"
+#include "util/parse.h"
 #include <ctype.h>
 #include <string.h>
 #include <stdlib.h>
 
-static int pax_read_decimal(const char *str, sqfs_u64 *out)
-{
-	sqfs_u64 result = 0;
-
-	while (*str >= '0' && *str <= '9') {
-		if (result > 0xFFFFFFFFFFFFFFFFUL / 10) {
-			fputs("numeric overflow parsing pax header\n", stderr);
-			return -1;
-		}
-
-		result = (result * 10) + (*(str++) - '0');
-	}
-
-	*out = result;
-	return 0;
-}
-
 static void urldecode(char *str)
 {
 	unsigned char *out = (unsigned char *)str;
@@ -95,6 +79,7 @@ static int pax_slink(tar_header_decoded_t *out, char *path)
 static int pax_sparse_map(tar_header_decoded_t *out, const char *line)
 {
 	sparse_map_t *last = NULL, *list = NULL, *ent = NULL;
+	size_t diff;
 
 	free_sparse_list(out->sparse);
 	out->sparse = NULL;
@@ -104,20 +89,17 @@ static int pax_sparse_map(tar_header_decoded_t *out, const char *line)
 		if (ent == NULL)
 			goto fail_errno;
 
-		if (pax_read_decimal(line, &ent->offset))
+		if (parse_uint(line, -1, &diff, 0, 0, &ent->offset))
 			goto fail_format;
 
-		while (isdigit(*line))
-			++line;
-
-		if (*(line++) != ',')
+		if (line[diff] != ',')
 			goto fail_format;
 
-		if (pax_read_decimal(line, &ent->count))
+		line += diff + 1;
+		if (parse_uint(line, -1, &diff, 0, 0, &ent->count))
 			goto fail_format;
 
-		while (isdigit(*line))
-			++line;
+		line += diff;
 
 		if (last == NULL) {
 			list = last = ent;
@@ -242,23 +224,17 @@ static int apply_handler(tar_header_decoded_t *out,
 	sqfs_xattr_t *xattr;
 	sqfs_s64 s64val;
 	sqfs_u64 uval;
+	size_t diff;
 	char *copy;
 
 	switch (field->type) {
 	case PAX_TYPE_SINT:
-		if (value[0] == '-') {
-			if (pax_read_decimal(value + 1, &uval))
-				return -1;
-			s64val = -((sqfs_s64)uval);
-		} else {
-			if (pax_read_decimal(value, &uval))
-				return -1;
-			s64val = (sqfs_s64)uval;
-		}
+		if (parse_int(value, -1, &diff, 0, 0, &s64val))
+			goto fail_numeric;
 		return field->cb.sint(out, s64val);
 	case PAX_TYPE_UINT:
-		if (pax_read_decimal(value, &uval))
-			return -1;
+		if (parse_uint(value, -1, &diff, 0, 0, &uval))
+			goto fail_numeric;
 		return field->cb.uint(out, uval);
 	case PAX_TYPE_CONST_STRING:
 		return field->cb.cstr(out, value);
@@ -291,6 +267,9 @@ static int apply_handler(tar_header_decoded_t *out,
 	}
 
 	return 0;
+fail_numeric:
+	fputs("Malformed decimal value in pax header\n", stderr);
+	return -1;
 }
 
 int read_pax_header(sqfs_istream_t *fp, sqfs_u64 entsize,
@@ -300,6 +279,7 @@ int read_pax_header(sqfs_istream_t *fp, sqfs_u64 entsize,
 	sparse_map_t *sparse_last = NULL, *sparse;
 	sqfs_u64 offset = 0, num_bytes = 0;
 	const struct pax_handler_t *field;
+	size_t diff;
 	long len;
 
 	buffer = record_to_memory(fp, entsize);
@@ -345,11 +325,11 @@ int read_pax_header(sqfs_istream_t *fp, sqfs_u64 entsize,
 
 			*set_by_pax |= field->flag;
 		} else if (!strcmp(key, "GNU.sparse.offset")) {
-			if (pax_read_decimal(value, &offset))
-				goto fail;
+			if (parse_uint(value, -1, &diff, 0, 0, &offset))
+				goto fail_malformed;
 		} else if (!strcmp(key, "GNU.sparse.numbytes")) {
-			if (pax_read_decimal(value, &num_bytes))
-				goto fail;
+			if (parse_uint(value, -1, &diff, 0, 0, &num_bytes))
+				goto fail_malformed;
 			sparse = calloc(1, sizeof(*sparse));
 			if (sparse == NULL)
 				goto fail_errno;
diff --git a/lib/util/Makemodule.am b/lib/util/Makemodule.am
index 6386066..9b2065a 100644
--- a/lib/util/Makemodule.am
+++ b/lib/util/Makemodule.am
@@ -9,7 +9,8 @@ libutil_a_SOURCES = include/util/util.h include/util/str_table.h \
 	lib/util/src/canonicalize_name.c lib/util/src/filename_sane.c \
 	lib/util/src/source_date_epoch.c lib/util/src/file_cmp.c \
 	lib/util/src/hex_decode.c lib/util/src/base64_decode.c \
-	lib/util/src/get_line.c lib/util/src/split_line.c
+	lib/util/src/get_line.c lib/util/src/split_line.c \
+	lib/util/src/parse_int.c
 libutil_a_CFLAGS = $(AM_CFLAGS)
 libutil_a_CPPFLAGS = $(AM_CPPFLAGS)
 
@@ -83,11 +84,14 @@ test_get_line_LDADD = libutil.a libio.a libcompat.a
 test_split_line_SOURCES = lib/util/test/split_line.c
 test_split_line_LDADD = libutil.a libcompat.a
 
+test_parse_int_SOURCES = lib/util/test/parse_int.c
+test_parse_int_LDADD = libutil.a libcompat.a
+
 LIBUTIL_TESTS = \
 	test_str_table test_rbtree test_xxhash test_threadpool test_ismemzero \
 	test_canonicalize_name test_filename_sane test_filename_sane_w32 \
 	test_sdate_epoch test_hex_decode test_base64_decode test_get_line \
-	test_split_line
+	test_split_line test_parse_int
 
 check_PROGRAMS += $(LIBUTIL_TESTS)
 TESTS += $(LIBUTIL_TESTS)
diff --git a/lib/util/src/parse_int.c b/lib/util/src/parse_int.c
new file mode 100644
index 0000000..1bca528
--- /dev/null
+++ b/lib/util/src/parse_int.c
@@ -0,0 +1,89 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/*
+ * alloc.c
+ *
+ * Copyright (C) 2019 David Oberhollenzer <goliath@infraroot.at>
+ */
+#include "config.h"
+
+#include "util/parse.h"
+#include "sqfs/error.h"
+
+#include <ctype.h>
+
+int parse_uint(const char *in, size_t len, size_t *diff,
+	       sqfs_u64 vmin, sqfs_u64 vmax, sqfs_u64 *out)
+{
+	/* init result */
+	if (diff != NULL)
+		*diff = 0;
+	*out = 0;
+
+	/* sequence has at least 1 digit */
+	if (len == 0 || !isdigit(*in))
+		return SQFS_ERROR_CORRUPTED;
+
+	/* parse sequence */
+	while (len > 0 && isdigit(*in)) {
+		sqfs_u64 x = *(in++) - '0';
+		--len;
+
+		if (diff != NULL)
+			++(*diff);
+
+		if ((*out) >= (0xFFFFFFFFFFFFFFFFULL / 10ULL))
+			return SQFS_ERROR_OVERFLOW;
+
+		(*out) *= 10;
+
+		if ((*out) > (0xFFFFFFFFFFFFFFFFULL - x))
+			return SQFS_ERROR_OVERFLOW;
+
+		(*out) += x;
+	}
+
+	/* range check */
+	if ((vmin != vmax) && ((*out < vmin) || (*out > vmax)))
+		return SQFS_ERROR_OUT_OF_BOUNDS;
+
+	/* if diff is not used, entire  must have been processed */
+	if (diff == NULL && (len > 0 && *in != '\0'))
+		return SQFS_ERROR_CORRUPTED;
+
+	return 0;
+}
+
+
+int parse_int(const char *in, size_t len, size_t *diff,
+	      sqfs_s64 vmin, sqfs_s64 vmax, sqfs_s64 *out)
+{
+	bool negative = false;
+	sqfs_u64 temp;
+	int ret;
+
+	if (len > 0 && *in == '-') {
+		++in;
+		--len;
+		negative = true;
+	}
+
+	ret = parse_uint(in, len, diff, 0, 0, &temp);
+	if (ret)
+		return ret;
+
+	if (temp >= 0x7FFFFFFFFFFFFFFFULL)
+		return SQFS_ERROR_OVERFLOW;
+
+	if (negative) {
+		if (diff != NULL)
+			(*diff) += 1;
+		*out = -((sqfs_s64)temp);
+	} else {
+		*out = (sqfs_s64)temp;
+	}
+
+	if (vmin != vmax && ((*out < vmin) || (*out > vmax)))
+		return SQFS_ERROR_OUT_OF_BOUNDS;
+
+	return 0;
+}
diff --git a/lib/util/test/parse_int.c b/lib/util/test/parse_int.c
new file mode 100644
index 0000000..2bd5a7c
--- /dev/null
+++ b/lib/util/test/parse_int.c
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-3.0-or-later */
+/*
+ * parse_int.c
+ *
+ * Copyright (C) 2019 David Oberhollenzer <goliath@infraroot.at>
+ */
+#include "config.h"
+#include "util/parse.h"
+#include "util/test.h"
+#include "sqfs/error.h"
+
+int main(int argc, char **argv)
+{
+	sqfs_s64 s_out;
+	sqfs_u64 out;
+	size_t diff;
+	int ret;
+	(void)argc; (void)argv;
+
+	/* must begin with a digit */
+	ret = parse_uint("a1234", -1, &diff, 0, 0, &out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_CORRUPTED);
+
+	/* can end with a non-digit... */
+	ret = parse_uint("1234a", -1, &diff, 0, 0, &out);
+	TEST_EQUAL_I(ret, 0);
+	TEST_EQUAL_UI(out, 1234);
+	TEST_EQUAL_UI(diff, 4);
+
+	/* ...unless diff is NULL */
+	ret = parse_uint("1234a", -1, NULL, 0, 0, &out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_CORRUPTED);
+
+	/* numeric overflow is cought */
+	ret = parse_uint("18446744073709551616", -1, NULL, 0, 0, &out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_OVERFLOW);
+
+	/* buffer length is adherered to */
+	ret = parse_uint("18446744073709551616", 5, NULL, 0, 0, &out);
+	TEST_EQUAL_I(ret, 0);
+	TEST_EQUAL_UI(out, 18446);
+
+	ret = parse_uint("18446744073709551616", 5, &diff, 0, 0, &out);
+	TEST_EQUAL_I(ret, 0);
+	TEST_EQUAL_UI(diff, 5);
+	TEST_EQUAL_UI(out, 18446);
+
+	/* if vmin/vmax differ, check the range */
+	ret = parse_uint("1234", -1, NULL, 0, 1000, &out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_OUT_OF_BOUNDS);
+
+	ret = parse_uint("1234", -1, NULL, 0, 2000, &out);
+	TEST_EQUAL_I(ret, 0);
+	TEST_EQUAL_UI(out, 1234);
+
+	ret = parse_uint("1234", -1, NULL, 2000, 3000, &out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_OUT_OF_BOUNDS);
+
+	/* int version accepts '-' prefix */
+	ret = parse_int("1234", -1, NULL, 0, 0, &s_out);
+	TEST_EQUAL_I(ret, 0);
+	TEST_EQUAL_I(s_out, 1234);
+
+	ret = parse_int("-1234", -1, NULL, 0, 0, &s_out);
+	TEST_EQUAL_I(ret, 0);
+	TEST_EQUAL_I(s_out, -1234);
+
+	ret = parse_int("- 1234", -1, NULL, 0, 0, &s_out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_CORRUPTED);
+
+	ret = parse_int("+1234", -1, NULL, 0, 0, &s_out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_CORRUPTED);
+
+	ret = parse_int("-1234", -1, NULL, -1000, 1000, &s_out);
+	TEST_EQUAL_I(ret, SQFS_ERROR_OUT_OF_BOUNDS);
+
+	return EXIT_SUCCESS;
+}