1. 22 Jan, 2019 1 commit
  2. 14 Nov, 2018 1 commit
    • strntol: fix out-of-bounds reads when parsing numbers with leading sign · 4209a512
      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
      Patrick Steinhardt committed
  3. 02 Nov, 2018 2 commits
    • strntol: fix detection and skipping of base prefixes · 50d09407
      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.
      Patrick Steinhardt committed
    • strntol: fix out-of-bounds read when skipping leading spaces · 41863a00
      The `git__strntol` family of functions accepts leading spaces and will
      simply skip them. The skipping will not honor the provided buffer's
      length, though, which may lead it to read outside of the provided
      buffer's bounds if it is not a simple NUL-terminated string.
      Furthermore, if leading space is trimmed, the function will further
      advance the pointer but not update the number of remaining bytes, which
      may also lead to out-of-bounds reads.
      
      Fix the issue by properly paying attention to the buffer length and
      updating it when stripping leading whitespace characters. Add a test
      that verifies that we won't read past the provided buffer length.
      Patrick Steinhardt committed
  4. 19 Oct, 2018 1 commit
    • util: fix out of bounds read in error message · ea19efc1
      When an integer that is parsed with `git__strntol32` is too big to fit
      into an int32, we will generate an error message that includes the
      actual string that failed to parse. This does not acknowledge the fact
      that the string may either not be NUL terminated or alternative include
      additional characters after the number that is to be parsed. We may thus
      end up printing characters into the buffer that aren't the number or,
      worse, read out of bounds.
      
      Fix the issue by utilizing the `endptr` that was set by
      `git__strntol64`. This pointer is guaranteed to be set to the first
      character following the number, and we can thus use it to compute the
      width of the number that shall be printed. Create a test to verify that
      we correctly truncate the number.
      Patrick Steinhardt committed
  5. 18 Oct, 2018 3 commits
  6. 29 Jun, 2016 1 commit
  7. 14 Nov, 2013 1 commit
  8. 25 Jan, 2012 1 commit
  9. 07 Oct, 2011 1 commit
    • tests-clay: update clay · 681008c7
      The clay script didn't match the latest version from upstream.
      Additionaly, add core/strtol.c to complete porting the core tests to
      clay.
      
      Signed-off-by: schu <schu-github@schulog.org>
      schu committed