From a64417804f4c2b0425e167851d10854cf1f23e99 Mon Sep 17 00:00:00 2001 From: David Oberhollenzer Date: Sat, 1 Jul 2023 12:41:56 +0200 Subject: 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 --- lib/common/src/parse_size.c | 42 ++++++++++----------- lib/tar/src/pax_header.c | 60 ++++++++++-------------------- lib/util/Makemodule.am | 8 +++- lib/util/src/parse_int.c | 89 +++++++++++++++++++++++++++++++++++++++++++++ lib/util/test/parse_int.c | 78 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 212 insertions(+), 65 deletions(-) create mode 100644 lib/util/src/parse_int.c create mode 100644 lib/util/test/parse_int.c (limited to 'lib') 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 */ #include "common.h" +#include "util/parse.h" #include #include @@ -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 #include #include -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 + */ +#include "config.h" + +#include "util/parse.h" +#include "sqfs/error.h" + +#include + +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 + */ +#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; +} -- cgit v1.2.3