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.

Author: pks-t

src/util.c

@@ -97,24 +97,35 @@ 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
 	 */

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;