Commit 50d0940760110ae9a8f340bd6e478ab12c01d791
2018-10-29T18:05:27
strntol: fix detection and skipping of base prefixes The `git__strntol` family of functions has the ability to auto-detect a number's base if the string has either the common '0x' prefix for hexadecimal numbers or '0' prefix for octal numbers. The detection of such prefixes and following handling has two major issues though that are being fixed in one go now. - We do not do any bounds checking previous to verifying the '0x' base. While we do verify that there is at least one digit available previously, we fail to verify that there are two digits available and thus may do an out-of-bounds read when parsing this two-character-prefix. - When skipping the prefix of such numbers, we only update the pointer length without also updating the number of remaining bytes. Thus if we try to parse a number '0x1' of total length 3, we will first skip the first two bytes and then try to read 3 bytes starting at '1'. Fix both issues by disentangling the logic. Instead of doing the detection and skipping of such prefixes in one go, we will now first try to detect the base while also honoring how many bytes are left. Only if we have a valid base that is either 8 or 16 and have one of the known prefixes, we will now advance the pointer and update the remaining bytes in one step. Add some tests that verify that no out-of-bounds parsing happens and that autodetection works as advertised.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85
diff --git a/src/util.c b/src/util.c
index b191d1a..735f0b5 100644
--- a/src/util.c
+++ b/src/util.c
@@ -97,25 +97,36 @@ int git__strntol64(int64_t *result, const char *nptr, size_t nptr_len, const cha
neg = 1;
/*
- * Base
+ * Automatically detect the base if none was given to us.
+ * Right now, we assume that a number starting with '0x'
+ * is hexadecimal and a number starting with '0' is
+ * octal.
*/
if (base == 0) {
if (*p != '0')
base = 10;
- else {
+ else if (nptr_len > 2 && (p[1] == 'x' || p[1] == 'X'))
+ base = 16;
+ else
base = 8;
- if (p[1] == 'x' || p[1] == 'X') {
- p += 2;
- base = 16;
- }
- }
- } else if (base == 16 && *p == '0') {
- if (p[1] == 'x' || p[1] == 'X')
- p += 2;
- } else if (base < 0 || 36 < base)
+ }
+
+ if (base < 0 || 36 < base)
goto Return;
/*
+ * Skip prefix of '0x'-prefixed hexadecimal numbers. There is no
+ * need to do the same for '0'-prefixed octal numbers as a
+ * leading '0' does not have any impact. Also, if we skip a
+ * leading '0' in such a string, then we may end up with no
+ * digits left and produce an error later on which isn't one.
+ */
+ if (base == 16 && nptr_len > 2 && p[0] == '0' && (p[1] == 'x' || p[1] == 'X')) {
+ p += 2;
+ nptr_len -= 2;
+ }
+
+ /*
* Non-empty sequence of digits
*/
for (; nptr_len > 0; p++,ndig++,nptr_len--) {
diff --git a/tests/core/strtol.c b/tests/core/strtol.c
index ac19a28..71af332 100644
--- a/tests/core/strtol.c
+++ b/tests/core/strtol.c
@@ -64,6 +64,28 @@ void test_core_strtol__int64(void)
assert_l64_fails("-0x8000000000000001", 16);
}
+void test_core_strtol__base_autodetection(void)
+{
+ assert_l64_parses("0", 0, 0);
+ assert_l64_parses("00", 0, 0);
+ assert_l64_parses("0x", 0, 0);
+ assert_l64_parses("0foobar", 0, 0);
+ assert_l64_parses("07", 7, 0);
+ assert_l64_parses("017", 15, 0);
+ assert_l64_parses("0x8", 8, 0);
+ assert_l64_parses("0x18", 24, 0);
+}
+
+void test_core_strtol__buffer_length_with_autodetection_truncates(void)
+{
+ int64_t i64;
+
+ cl_git_pass(git__strntol64(&i64, "011", 2, NULL, 0));
+ cl_assert_equal_i(i64, 1);
+ cl_git_pass(git__strntol64(&i64, "0x11", 3, NULL, 0));
+ cl_assert_equal_i(i64, 1);
+}
+
void test_core_strtol__buffer_length_truncates(void)
{
int32_t i32;