tests


Log

Author Commit Date CI Message
Patrick Steinhardt bc219657 2018-12-19T11:01:55 Merge pull request #4833 from csware/drop-empty-dirs Remove empty (sub-)directories when deleting refs
Carlos Martín Nieto 0f299365 2018-12-14T14:29:36 annotated_commit: add failing test for looking up from annotated tag
Edward Thomson 168fe39b 2018-11-28T14:26:57 object_type: use new enumeration names Use the new object_type enumeration names within the codebase.
Edward Thomson 18e71e6d 2018-11-28T13:31:06 index: use new enum and structure names Use the new-style index names throughout our own codebase.
Patrick Steinhardt 0ddc6094 2018-11-30T09:46:14 Merge pull request #4770 from tiennou/feature/merge-analysis-any-branch Allow merge analysis against any reference
Patrick Steinhardt e7873eb2 2018-11-29T08:00:31 Merge pull request #4888 from TheBB/add-cb revwalk: Allow changing hide_cb
Patrick Steinhardt 487233fa 2018-11-29T07:21:41 Merge pull request #4895 from pks-t/pks/unused-warnings Unused function warnings
Edward Thomson a904fc6d 2018-11-28T20:31:30 Merge pull request #4870 from libgit2/ethomson/proxy Add builtin proxy support for the http transport
Edward Thomson 02bb39f4 2018-11-22T08:49:09 stream registration: take an enum type Accept an enum (`git_stream_t`) during custom stream registration that indicates whether the registration structure should be used for standard (non-TLS) streams or TLS streams.
Edward Thomson 43b592ac 2018-10-25T08:49:01 tls: introduce a wrap function Introduce `git_tls_stream_wrap` which will take an existing `stream` with an already connected socket and begin speaking TLS on top of it. This is useful if you've built a connection to a proxy server and you wish to begin CONNECT over it to tunnel a TLS connection. Also update the pluggable TLS stream layer so that it can accept a registration structure that provides an `init` and `wrap` function, instead of a single initialization function.
Edward Thomson 6ba3e6af 2018-11-18T21:53:48 proxy tests: rename credential callback Rename credential callback to proxy_cred_cb to match new cert callback.
Edward Thomson 394ae7e1 2018-10-22T17:35:35 proxy tests: support self-signed proxy cert Give the proxy tests a proxy certificate callback, and allow self-signed certificates when the `GITTEST_REMOTE_PROXY_SELFSIGNED` environment variable is set (to anything). In that case, simply compare the hostname from the callback to the hostname that we connected to.
Edward Thomson df2cc108 2018-11-18T10:29:07 stream: provide generic registration API Update the new stream registration API to be `git_stream_register` which takes a registration structure and a TLS boolean. This allows callers to register non-TLS streams as well as TLS streams. Provide `git_stream_register_tls` that takes just the init callback for backward compatibliity.
Edward Thomson 45054732 2018-10-29T10:45:59 tests: optionally ignore https cert validation For testing, we may wish to use a man-in-the-middle proxy that can inspect the CONNECT traffic to our test endpoints. For this, we will need to accept the proxy's certificate, which will not be valid for the true endpoint. Add a new environment variable, GITTEST_REMOTE_SSL_NOVERIFY to disable https certificate validation for the tests.
Edward Thomson 4ecc14cd 2018-10-21T23:47:53 tests: support optional PROXY_SCHEME As we want to support HTTPS proxies, support an optional `GITTEST_REMOTE_PROXY_SCHEME` environment variable for tests that will allow for HTTPS support. (When unset, the tests default to HTTP proxies.)
Edward Thomson de60d9b4 2018-10-21T21:00:37 tests: PROXY_URL is more accurately PROXY_HOST Change the `GITTEST_REMOTE_PROXY_URL` environment variable to be `GITTEST_REMOTE_PROXY_HOST`, since it is a host:port combination, not an actual URL. (We cannot use a URL here since we may want to include the username:password combination in the constructed URL.)
Patrick Steinhardt 14a9a4f3 2018-11-21T11:18:46 tests: move apply_helpers functions into own compilation unit Currently, the "apply_helper" functions used for testing the apply logic are all statically defined in the "apply_helpers.h" header file. This may lead to warnings from the compiler in case where this header file is included, but not all functions it brings along are used in the compilation unit where it has been included into. Fix these potential warnings by moving the implementation into its own compilation unit "apply_helpers.c".
Patrick Steinhardt 2521e11c 2018-11-23T20:02:31 tests: mailmap: avoid definition of unused static variables The mailmap testdata header contains a set of static variable definitions. As these variables aren't used in all places where they are used, they trigger the unused-const-variable warnings. As we have currently disabled those warnings explicitly, they are never triggered, but we intend to enable them. Avoid the issue by only keeping variable definitions that are actually used in all locations. Move the others to where they are used.
Patrick Steinhardt 852bc9f4 2018-11-23T19:26:24 khash: remove intricate knowledge of khash types Instead of using the `khiter_t`, `git_strmap_iter` and `khint_t` types, simply use `size_t` instead. This decouples code from the khash stuff and makes it possible to move the khash includes into the implementation files.
Patrick Steinhardt bbf9f5a7 2018-11-21T11:20:14 tests: path: only compile test_canonicalize on Win32 platforms The function `test_canonicalize` is only used on Win32 platforms. It will thus result in an unused function warning if these warnings are enabled and one is on a platform different than Win32. Fix the issue by only compiling in the function on Win32 platforms.
Eivind Fonn 0836f069 2018-11-14T16:08:30 revwalk: Allow changing hide_cb Since git_revwalk objects are encouraged to be reused, a public interface for changing hide_cb is desirable.
Patrick Steinhardt 43cbe6b7 2018-11-28T13:36:47 config: fix adding files if their parent directory is a file When we try to add a configuration file with `git_config_add_file_ondisk`, we treat nonexisting files as empty. We do this by performing a stat call, ignoring ENOENT errors. This works just fine in case the file or any of its parents simply does not exist, but there is also the case where any of the parent directories is not a directory, but a file. So e.g. trying to add a configuration file "/dev/null/.gitconfig" will fail, as `errno` will be ENOTDIR instead of ENOENT. Catch ENOTDIR in addition to ENOENT to fix the issue. Add a test that verifies we are able to add configuration files with such an invalid path file just fine.
Patrick Steinhardt 0e3e832d 2018-11-21T13:30:01 Merge pull request #4884 from libgit2/ethomson/index_iterator index: introduce git_index_iterator
Edward Thomson 11d33df8 2018-11-18T23:39:43 Merge branch 'tiennou/fix/logallrefupdates-always'
Etienne Samson e226ad8f 2018-11-17T17:55:10 refs: add support for core.logAllRefUpdates=always Since we were not expecting this config entry to contain a string, we would fail as soon as its (cached) value would be accessed. Hence, provide some constants for the 4 states we use, and account for "always" when we decide to reflog changes.
Edward Thomson 646a94be 2018-11-18T23:15:56 Merge pull request #4847 from noahp/noahp/null-arg-fixes tests: 🌀 address two null argument instances
Edward Thomson 7321cff0 2018-11-15T09:17:51 Merge pull request #4713 from libgit2/ethomson/win_symlinks Support symlinks on Windows when core.symlinks=true
Edward Thomson c358bbc5 2018-11-12T17:22:47 index: introduce git_index_iterator Provide a public git_index_iterator API that is backed by an index snapshot. This allows consumers to provide a stable iteration even while manipulating the index during iteration.
Patrick Steinhardt 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
Edward Thomson fd4e3b21 2018-11-13T15:33:20 Merge pull request #4885 from pks-t/pks/apply-test-fixups apply: small fixups in the test suite
Patrick Steinhardt cf83809b 2018-11-13T14:26:26 Merge pull request #4883 from pks-t/pks/signature-tz-oob signature: fix out-of-bounds read when parsing timezone offset
Noah Pendleton f127ce35 2018-11-13T08:22:25 tests: address two null argument instances Handle two null argument cases that occur in the unit tests. One is in library code, the other is in test code. Detected by running unit tests with undefined behavior sanitizer: ```bash # build mkdir build && cd build cmake -DBUILD_CLAR=ON -DCMAKE_C_FLAGS="-fsanitize=address \ -fsanitize=undefined -fstack-usage -static-libasan" .. cmake --build . # run with asan ASAN_OPTIONS="allocator_may_return_null=1" ./libgit2_clar ... ............../libgit2/src/apply.c:316:3: runtime error: null pointer \ passed as argument 1, which is declared to never be null ...................../libgit2/tests/apply/fromfile.c:46:3: runtime \ error: null pointer passed as argument 1, which is declared to never be null ```
Patrick Steinhardt afc64bcd 2018-11-13T14:13:40 tests: apply: fix reference to deprecated `git_buf_free` Since commit 56ffdfc61 (buffer: deprecate `git_buf_free` in favor of `git_buf_dispose`, 2018-02-08), the function `git_buf_free` is deprecated and shall not be used anymore. As part of the new apply framework that has been cooking for quite some time some new references have been introduced to that deprecated function. Replace them with calls to `git_buf_dispose`.
Patrick Steinhardt fe215153 2018-11-13T14:08:49 tests: apply: fix missing `cl_git_pass` wrappers Some function calls in the new "apply" test suite were missing the checks whether they succeeded as expected. Fix this by adding the missing `cl_git_pass` wrappers.
Patrick Steinhardt 20cb30b6 2018-11-13T13:40:17 Merge pull request #4667 from tiennou/feature/remote-create-api Remote creation API
Patrick Steinhardt 28239be3 2018-11-13T13:27:41 Merge pull request #4818 from pks-t/pks/index-collision Index collision fixes
Edward Thomson 11fbead8 2018-11-11T16:40:56 Merge pull request #4705 from libgit2/ethomson/apply Patch (diff) application
Patrick Steinhardt 52f859fd 2018-11-09T19:32:08 signature: fix out-of-bounds read when parsing timezone offset When parsing a signature's timezone offset, we first check whether there is a timezone at all by verifying that there are still bytes left to read following the time itself. The check thus looks like `time_end + 1 < buffer_end`, which is actually correct in this case. After setting the timezone's start pointer to that location, we compute the remaining bytes by using the formula `buffer_end - tz_start + 1`, re-using the previous `time_end + 1`. But this is in fact missing the braces around `(tz_start + 1)`, thus leading to an overestimation of the remaining bytes by a length of two. In case of a non-NUL terminated buffer, this will result in an overflow. The function `git_signature__parse` is only used in two locations. First is `git_signature_from_buffer`, which only accepts a string without a length. The string thus necessarily has to be NUL terminated and cannot trigger the issue. The other function is `git_commit__parse_raw`, which can in fact trigger the error as it may receive non-NUL terminated commit data. But as objects read from the ODB are always NUL-terminated by us as a cautionary measure, it cannot trigger the issue either. In other words, this error does not have any impact on security.
Edward Thomson 4e746d80 2018-11-05T15:49:11 test: ensure applying a patch can't delete a file twice
Edward Thomson f8b9493b 2018-11-05T15:46:08 apply: test re-adding a file after removing it Ensure that we can add a file back after it's been removed. Update the renamed/deleted validation in application to not apply to deltas that are adding files to support this.
Edward Thomson 78580ad3 2018-11-05T15:34:59 apply: test modifying a file after renaming it Ensure that we cannot modify a file after it's been renamed out of the way. If multiple deltas exist for a single path, ensure that we do not attempt to modify a file after it's been renamed out of the way. To support this, we must track the paths that have been removed or renamed; add to a string map when we remove a path and remove from the string map if we recreate a path. Validate that we are not applying to a path that is in this map, unless the delta is a rename, since git supports renaming one file to two different places in two different deltas. Further, test that we cannot apply a modification delta to a path that will be created in the future by a rename (a path that does not yet exist.)
Edward Thomson 605066ee 2018-11-05T14:37:35 apply: test renaming a file after modifying it Multiple deltas can exist in a diff, and can be applied in-order. If there exists a delta that modifies a file followed by a delta that renames that file, then both will be captured. The modification delta will be applied and the resulting file will be staged with the original filename. The rename delta will be independently applied - to the original file (not the modified file from the original delta) and staged independently.
Edward Thomson bd682f3e 2018-11-04T19:01:57 apply: test that we can't rename a file after modifying it Multiple deltas can exist in a diff, and can be applied in-order. However if there exists a delta that renames a file, it must be first, so that other deltas can reference the resulting target file. git enforces this (`error: already exists in index`), so ensure that we do, too.
Edward Thomson a3c1070c 2018-11-04T14:07:22 apply: test modify delta after rename delta Ensure that we can apply a delta after renaming a file.
Edward Thomson 07e71bfa 2018-11-04T13:14:20 apply: test multiple deltas to new file
Edward Thomson df4258ad 2018-11-04T13:01:03 apply: handle multiple deltas to the same file git allows a patch file to contain multiple deltas to the same file: although it does not produce files in this format itself, this could be the result of concatenating two different patch files that affected the same file. git apply behaves by applying this next delta to the existing postimage of the file. We should do the same. If we have previously seen a file, and produced a postimage for it, we will load that postimage and apply the current delta to that. If we have not, get the file from the preimage.
Edward Thomson c71e964a 2018-11-04T12:21:57 apply: test rename 1 to 2 Test that a patch can contain two deltas that appear to rename an initial source file to two different destination paths. Git creates both target files with the initial source contents; ensure that we do, too.
Edward Thomson 56a2ae0c 2018-11-04T12:18:01 apply: test rename 2 to 1 Test that we can apply a patch that renames two different files to the same target filename. Git itself handles this scenario in a last-write wins, such that the rename listed last is the one persisted in the target. Ensure that we do the same.
Edward Thomson 235dc9b2 2018-11-04T12:05:46 apply: test circular rename Test a rename from A->B simultaneous with a rename from B->A.
Edward Thomson 89b5a56e 2018-11-04T11:58:20 apply: test rename A -> B -> C scenarios Test that we can rename some file from B->C and then rename some other file from A->B. Do this with both exact rename patches (eg `rename from ...` / `rename to ...`) and patches that remove the files and replace them entirely.
Edward Thomson 6fecf4d1 2018-11-04T11:47:46 apply: handle exact renames Deltas containing exact renames are special; they simple indicate that a file was renamed without providing additional metadata (like the filemode). Teach the reader to provide the file mode and use the preimage's filemode in the case that the delta does not provide one.)
Edward Thomson 12f9ac17 2018-11-04T11:26:42 apply: validate unchanged mode when applying both When applying to both the index and the working directory, ensure that the working directory's mode matches the index's mode. It's not sufficient to look only at the hashed object id to determine that the file is unchanged, git also takes the mode into account.
Edward Thomson b73a42f6 2018-11-04T10:48:23 apply: test a patch with rename and modification Create a test applying a patch with a rename and a modification of a file.
Jason Haslam 620ac9c2 2017-04-11T14:41:57 patch: add tests for aborting hunk callback
Jason Haslam 72630572 2017-03-30T22:40:47 patch: add support for partial patch application Add hunk callback parameter to git_apply__patch to allow hunks to be skipped.
Edward Thomson 47cc5f85 2018-09-29T19:32:51 apply: introduce a hunk callback Introduce a callback to patch application that allows consumers to cancel hunk application.
Edward Thomson 398d8bfe 2018-07-16T17:19:08 apply tests: tests a diff w/ many small changes
Edward Thomson b8840db7 2018-07-10T16:18:45 apply tests: test delta callback skip Test that we can return a non-zero value from the apply delta callback and it will skip the application of a given delta.
Edward Thomson db6b1164 2018-07-10T16:13:17 apply tests: test delta callback errors Test that we can return an error from the apply delta callback and the error code is propagated back to the caller.
Edward Thomson 37b25ac5 2018-07-08T16:12:58 apply: move location to an argument, not the opts Move the location option to an argument, out of the options structure. This allows the options structure to be re-used for functions that don't need to know the location, since it's implicit in their functionality. For example, `git_apply_tree` should not take a location, but is expected to take all the other options.
Edward Thomson 813f0802 2018-07-01T15:14:36 apply: validate workdir contents match index for BOTH When applying to both the index and the working directory, ensure that the index contents match the working directory. This mirrors the requirement in `git apply --index`. This also means that - along with the prior commit that uses the working directory contents as the checkout baseline - we no longer expect conflicts during checkout. So remove the special-case error handling for checkout conflicts. (Any checkout conflict now would be because the file was actually modified between the start of patch application and the checkout.)
Edward Thomson 3b674660 2018-07-01T13:46:59 apply tests: ensure we can patch a modified file Patch application need not be on an unmodified file; applying to an already changed file is supported provided the patch still applies cleanly. Add tests that modifies the contents of a file then applies the patch and ensures that the patch applies cleanly, and the original changes are also kept.
Edward Thomson eb76e985 2018-07-01T21:21:25 apply tests: ensure mode changes occur Test that a mode change is reflected in the working directory or index.
Edward Thomson 5c63ce79 2018-07-01T11:10:03 apply tests: test with CR/LF filtering Ensure that we accurately CR/LF filter when reading from the working directory. If we did not, we would erroneously fail to apply the patch because the index contents did not match the working directory contents.
Edward Thomson 4ff829e9 2018-06-30T17:20:03 apply tests: test index+workdir application Test application with `GIT_APPLY_LOCATION_BOTH`, which emulates `git apply --index`, updating both the index and the working directory with the postimage.
Edward Thomson 9db66c79 2018-06-29T12:50:38 apply test: apply with non-conflicting changes Ensure that we can apply to the working directory or the index when the application target is modified, so long as there are not conflicting changes to the items.
Edward Thomson 771bd81e 2018-06-29T12:40:16 apply tests: ensure apply failures leave index unmodified
Edward Thomson 2bd3cfea 2018-06-29T11:43:55 apply tests: modified wd items are ok when applying to index When applying to the index (using `GIT_APPLY_LOCATION_INDEX`), ensure that items modified in the working directory do not conflict with the application.
Edward Thomson d7090ee4 2018-06-28T17:26:24 apply tests: ensure we can add and remove files from the index Add a test that adds a new file, and another that removes a file when applying using `GIT_APPLY_LOCATION_INDEX` to ensure that they work.
Edward Thomson 9d81defa 2018-06-28T16:26:08 apply tests: GIT_APPLY_LOCATION_INDEX with parsed patches
Edward Thomson eef34e4e 2018-06-28T16:24:21 apply tests: GIT_APPLY_LOCATION_INDEX with generated patches Test a simple patch application with `GIT_APPLY_LOCATION_INDEX`, which emulates `git apply --cached`.
Edward Thomson c010c93b 2018-06-27T16:50:07 apply tests: move helpers into common area
Edward Thomson 35d525b0 2018-06-26T09:19:12 apply: test that failures don't dirty workdir Ensure that when a patch application fails (due to a conflict in the working directory, for example) that we do not half-apply the patch or otherwise leave the working directory dirty. This is rather obvious in our current apply implementation (we do a two step process: one to create the post-image and one to check it out) but this test is a safety net for future refactoring or improvements.
Edward Thomson 973bf0c8 2018-06-25T20:49:22 apply: test a patch can be applied even with a modified index Ensure that we can apply a patch to the working directory, even to files that are modified in the index (as long as the working directory contents match the preimage - such that the working directory is unmodified from HEAD).
Edward Thomson 553395dc 2018-06-25T20:21:01 apply: test that the index is not modified Ensure that by default, when using GIT_APPLY_LOCATION_WORKDIR, that patch application does not update the index, only the working directory.
Edward Thomson 0eb63b9f 2018-06-25T19:50:35 apply tests: separate common patch hunks Move the commonly-used patch hunks into a single constant location. This allows us to avoid re-declaring them in each test, and allows us to compose them to build a larger patch file that includes all the hunks.
Edward Thomson 702d4bec 2018-06-26T15:26:37 apply tests: use `git_iterator_foreach` for tests Use the new `git_iterator_foreach` API to validate the workdir against the expected workdir values instead of using the paired/multi iterator comparison callback. This allows us to use the `git_iterator_foreach` to validate the index as well, instead of assuming that the index and HEAD must always match.
Edward Thomson 9c34c996 2018-06-25T17:03:14 apply: handle file additions Don't attempt to read the postimage file during a file addition, simply use an empty buffer as the postimage. Also, test that we can handle file additions.
Edward Thomson 3b5378c5 2018-06-25T16:27:06 apply: handle file deletions If the file was deleted in the postimage, do not attempt to update the target. Instead, ignore it and simply allow it to stay removed in our computed postimage. Also, test that we can handle file deletions.
Edward Thomson af3287f8 2018-06-22T19:27:19 apply: test `git_apply` with a parsed patch Ensure that we can apply a simple patch to the working directory when we have parsed it from a patch file.
Edward Thomson ff296b71 2018-03-19T19:50:52 apply: test `git_apply` application to a workdir Introduce a standard test applying a diff to a working directory with no complications.
Edward Thomson 02b1083a 2018-01-28T23:25:07 apply: introduce `git_apply_tree` Introduce `git_apply_tree`, which will apply a `git_diff` to a given `git_tree`, allowing an in-memory patch application for a repository.
Edward Thomson 2b12dcf6 2018-03-19T19:45:11 iterator: optionally hash filesystem iterators Optionally hash the contents of files encountered in the filesystem or working directory iterators. This is not expected to be used in production code paths, but may allow us to simplify some test contexts. For working directory iterators, apply filters as appropriate, since we have the context able to do it.
Etienne Samson 666c7bd8 2018-10-08T20:51:45 tests: unwarranted NULL-ification
Etienne Samson 3652b83a 2018-06-22T21:36:01 tests: remote/create: remove macro and unroll tests
Etienne Samson d3650294 2018-06-20T02:27:14 remote: add a flag to prevent generation of the default fetchspec
Etienne Samson fdb116b3 2018-06-20T02:27:12 remote: add a creation flag for ignoring url.insteadOf
Etienne Samson 3cbaebdf 2018-06-20T02:27:11 remote: provide a generic API for creating remotes This supersedes the functionality of remote_create_with_fetchspec, remote_create_anonymous and remote_create_detached.
Etienne Samson 0e5a27cd 2018-06-20T02:26:58 tests: count config section helper already exists
Etienne Samson f8fc987c 2018-06-20T02:26:56 tests: git_remote_create_detached
Etienne Samson 4e0da450 2018-06-20T02:26:55 tests: check what happens with the remote. section counts
Etienne Samson f778af68 2018-06-20T02:26:53 tests: git_remote_create_anonymous
Etienne Samson fa69195e 2018-06-20T02:26:52 tests: git_remote_create_with_fetchspec
Etienne Samson 10fa2dd6 2018-06-20T02:26:50 tests: consolidate all remote creation tests in one test suite
Etienne Samson 798be87e 2018-06-20T02:26:49 tests: rename remote creation test suite
Patrick Steinhardt 7fafec0e 2018-10-29T18:32:39 tree: fix integer overflow when reading unreasonably large filemodes The `parse_mode` option uses an open-coded octal number parser. The parser is quite naive in that it simply parses until hitting a character that is not in the accepted range of '0' - '7', completely ignoring the fact that we can at most accept a 16 bit unsigned integer as filemode. If the filemode is bigger than UINT16_MAX, it will thus overflow and provide an invalid filemode for the object entry. Fix the issue by using `git__strntol32` instead and doing a bounds check. As this function already handles overflows, it neatly solves the problem. Note that previously, `parse_mode` was also skipping the character immediately after the filemode. In proper trees, this should be a simple space, but in fact the parser accepted any character and simply skipped over it. As a consequence of using `git__strntol32`, we now need to an explicit check for a trailing whitespace after having parsed the filemode. Because of the newly introduced error message, the test object::tree::parse::mode_doesnt_cause_oob_read needs adjustment to its error message check, which in fact is a good thing as it demonstrates that we now fail looking for the whitespace immediately following the filemode. Add a test that shows that we will fail to parse such invalid filemodes now.
Patrick Steinhardt f647bbc8 2018-10-29T17:25:09 tree: fix mode parsing reading out-of-bounds When parsing a tree entry's mode, we will eagerly parse until we hit a character that is not in the accepted set of octal digits '0' - '7'. If the provided buffer is not a NUL terminated one, we may thus read out-of-bounds. Fix the issue by passing the buffer length to `parse_mode` and paying attention to it. Note that this is not a vulnerability in our usual code paths, as all object data read from the ODB is NUL terminated.
Patrick Steinhardt d4ad658a 2018-10-29T17:24:47 tree: add various tests exercising the tree parser We currently don't have any tests that directly exercise the tree parser. This is due to the fact that the parsers for raw object data has only been recently introduce with commit ca4db5f4a (object: implement function to parse raw data, 2017-10-13), and previous to that the setup simply was too cumbersome as it always required going through the ODB. Now that we have the infrastructure, add a suite of tests that directly exercise the tree parser and various edge cases.
Patrick Steinhardt 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.
Patrick Steinhardt 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.