|
f673e232
|
2018-12-27T13:47:34
|
|
git_error: use new names in internal APIs and usage
Move to the `git_error` name in the internal API for error-related
functions.
|
|
4209a512
|
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
|
|
50d09407
|
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.
|
|
41863a00
|
2018-10-29T17:19:58
|
|
strntol: fix out-of-bounds read when skipping leading spaces
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.
|
|
ea19efc1
|
2018-10-18T15:08:56
|
|
util: fix out of bounds read in error message
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.
|
|
39087ab8
|
2018-10-18T12:11:33
|
|
tests: core::strtol: test for some more edge-cases
Some edge cases were currently completely untested, e.g. parsing numbers
greater than INT64_{MIN,MAX}, truncating buffers by length and invalid
characters. Add tests to verify that the system under test performs as
expected.
|
|
8d7fa88a
|
2018-10-18T12:04:07
|
|
util: remove `git__strtol32`
The function `git__strtol32` can easily be misused when untrusted data
is passed to it that may not have been sanitized with trailing `NUL`
bytes. As all usages of this function have now been removed, we can
remove this function altogether to avoid future misuse of it.
|
|
68deb2cc
|
2018-10-18T11:37:10
|
|
util: remove unsafe `git__strtol64` function
The function `git__strtol64` does not take a maximum buffer length as
parameter. This has led to some unsafe usages of this function, and as
such we may consider it as being unsafe to use. As we have now
eradicated all usages of this function, let's remove it completely to
avoid future misuse.
|
|
70b9b841
|
2016-06-28T20:19:52
|
|
Fixed bug while parsing INT64_MIN
|
|
17820381
|
2013-11-14T14:05:52
|
|
Rename tests-clar to tests
|