thodg/libgit2/tests/core/strtol.c

• Show log

  Commit

• Hash : 4209a512
  Author :
  Date : 2018-11-14T12:04:42
  

  strntol: fix out-of-bounds reads when parsing numbers with leading sign
  
  When parsing a number, we accept a leading plus or minus sign to return
  a positive or negative number. When the parsed string has such a leading
  sign, we set up a flag indicating that the number is negative and
  advance the pointer to the next character in that string. This misses
  updating the number of bytes in the string, though, which is why the
  parser may later on do an out-of-bounds read.
  
  Fix the issue by correctly updating both the pointer and the number of
  remaining bytes. Furthermore, we need to check whether we actually have
  any bytes left after having advanced the pointer, as otherwise the
  auto-detection of the base may do an out-of-bonuds access. Add a test
  that detects the out-of-bound read.
  
  Note that this is not actually security critical. While there are a lot
  of places where the function is called, all of these places are guarded
  or irrelevant:
  
  - commit list: this operates on objects from the ODB, which are always
    NUL terminated any may thus not trigger the off-by-one OOB read.
  
  - config: the configuration is NUL terminated.
  
  - curl stream: user input is being parsed that is always NUL terminated
  
  - index: the index is read via `git_futils_readbuffer`, which always NUL
    terminates it.
  
  - loose objects: used to parse the length from the object's header. As
    we check previously that the buffer contains a NUL byte, this is safe.
  
  - rebase: this parses numbers from the rebase instruction sheet. As the
    rebase code uses `git_futils_readbuffer`, the buffer is always NUL
    terminated.
  
  - revparse: this parses a user provided buffer that is NUL terminated.
  
  - signature: this parser the header information of objects. As objects
    read from the ODB are always NUL terminated, this is a non-issue. The
    constructor `git_signature_from_buffer` does not accept a length
    parameter for the buffer, so the buffer needs to be NUL terminated, as
    well.
  
  - smart transport: the buffer that is parsed is NUL terminated
  
  - tree cache: this parses the tree cache from the index extension. The
    index itself is read via `git_futils_readbuffer`, which always NUL
    terminates it.
  
  - winhttp transport: user input is being parsed that is always NUL
    terminated
  

Download

• tests/core/strtol.c

• #include "clar_libgit2.h"
  
  static void assert_l32_parses(const char *string, int32_t expected, int base)
  {
  	int32_t i;
  	cl_git_pass(git__strntol32(&i, string, strlen(string), NULL, base));
  	cl_assert_equal_i(i, expected);
  }
  
  static void assert_l32_fails(const char *string, int base)
  {
  	int32_t i;
  	cl_git_fail(git__strntol32(&i, string, strlen(string), NULL, base));
  }
  
  static void assert_l64_parses(const char *string, int64_t expected, int base)
  {
  	int64_t i;
  	cl_git_pass(git__strntol64(&i, string, strlen(string), NULL, base));
  	cl_assert_equal_i(i, expected);
  }
  
  static void assert_l64_fails(const char *string, int base)
  {
  	int64_t i;
  	cl_git_fail(git__strntol64(&i, string, strlen(string), NULL, base));
  }
  
  void test_core_strtol__int32(void)
  {
  	assert_l32_parses("123", 123, 10);
  	assert_l32_parses("  +123 ", 123, 10);
  	assert_l32_parses("  +2147483647 ", 2147483647, 10);
  	assert_l32_parses("  -2147483648 ", -2147483648LL, 10);
  	assert_l32_parses("A", 10, 16);
  	assert_l32_parses("1x1", 1, 10);
  
  	assert_l32_fails("", 10);
  	assert_l32_fails("a", 10);
  	assert_l32_fails("x10x", 10);
  	assert_l32_fails("  2147483657 ", 10);
  	assert_l32_fails("  -2147483657 ", 10);
  }
  
  void test_core_strtol__int64(void)
  {
  	assert_l64_parses("123", 123, 10);
  	assert_l64_parses("  +123 ", 123, 10);
  	assert_l64_parses("  +2147483647 ", 2147483647, 10);
  	assert_l64_parses("  -2147483648 ", -2147483648LL, 10);
  	assert_l64_parses("  2147483657 ", 2147483657LL, 10);
  	assert_l64_parses("  -2147483657 ", -2147483657LL, 10);
  	assert_l64_parses(" 9223372036854775807  ", INT64_MAX, 10);
  	assert_l64_parses("   -9223372036854775808  ", INT64_MIN, 10);
  	assert_l64_parses("   0x7fffffffffffffff  ", INT64_MAX, 16);
  	assert_l64_parses("   -0x8000000000000000   ", INT64_MIN, 16);
  	assert_l64_parses("1a", 26, 16);
  	assert_l64_parses("1A", 26, 16);
  
  	assert_l64_fails("", 10);
  	assert_l64_fails("a", 10);
  	assert_l64_fails("x10x", 10);
  	assert_l64_fails("0x8000000000000000", 16);
  	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;
  	int64_t i64;
  
  	cl_git_pass(git__strntol32(&i32, "11", 1, NULL, 10));
  	cl_assert_equal_i(i32, 1);
  
  	cl_git_pass(git__strntol64(&i64, "11", 1, NULL, 10));
  	cl_assert_equal_i(i64, 1);
  }
  
  void test_core_strtol__buffer_length_with_leading_ws_truncates(void)
  {
  	int64_t i64;
  
  	cl_git_fail(git__strntol64(&i64, " 1", 1, NULL, 10));
  
  	cl_git_pass(git__strntol64(&i64, " 11", 2, NULL, 10));
  	cl_assert_equal_i(i64, 1);
  }
  
  void test_core_strtol__buffer_length_with_leading_sign_truncates(void)
  {
  	int64_t i64;
  
  	cl_git_fail(git__strntol64(&i64, "-1", 1, NULL, 10));
  
  	cl_git_pass(git__strntol64(&i64, "-11", 2, NULL, 10));
  	cl_assert_equal_i(i64, -1);
  }
  
  void test_core_strtol__error_message_cuts_off(void)
  {
  	assert_l32_fails("2147483657foobar", 10);
  	cl_assert(strstr(giterr_last()->message, "2147483657") != NULL);
  	cl_assert(strstr(giterr_last()->message, "foobar") == NULL);
  }
  

Download