| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The "trailer.c" code has been copied mostly verbatim from git.git with
minor adjustments, only. As git.git's `xmalloc` function, which aborts
on memory allocation errors, has been swapped out for `git_malloc`,
which doesn't abort, we may inadvertently access `NULL` pointers.
Add checks to fix this.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
In "posix.c" there are multiple callsites which execute `malloc` instead
of `git__malloc`. Thus, users of library are not able to track these
allocations with a custom allocator.
Convert these call sites to use `git__malloc` instead.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
When adding OIDs to the indexer's map of yet-to-be-seen OIDs to verify
that packfiles are complete, we do so by first allocating a new OID and
then calling `git_oidmap_set` on it. There was no check for memory
allocation errors in place, though, leading to possible segfaults due to
trying to copy data to a `NULL` pointer.
Verify the result of `git__malloc` with `GIT_ERROR_CHECK_ALLOC` to fix
the issue.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The function `git_commit_list_insert` dynamically allocates memory and
may thus fail to insert a given commit, but we didn't check for that in
several places in "merge.c".
Convert surrounding functions to return error codes and check whether
`git_commit_list_insert` was successful, returning an error if not.
|
| |_|_|_|/ /
|/| | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
The code in "blame_git.c" was mostly imported from git.git with only
minor changes. One of these changes was to use our own allocators
instead of git's `xmalloc`, but there's a subtle difference: `xmalloc`
would abort the program if unable to allocate any memory, bit
`git__malloc` doesn't. As we didn't check for memory allocation errors
in some places, we might inadvertently dereference a `NULL` pointer in
out-of-memory situations.
Convert multiple functions to return proper error codes and add calls to
`GIT_ERROR_CHECK_ALLOC` to fix this.
|
|\ \ \ \ \ \
| |_|/ / / /
|/| | | | | |
clone: don't decode URL percent encodings
|
| | | | | | |
|
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
The function `commit_quick_parse` provides a way to quickly parse
parts of a commit without storing or verifying most of its
metadata. The first thing it does is calculating the number of
parents by skipping "parent " lines until it finds the first
non-parent line. Afterwards, this parent count is passed to
`alloc_parents`, which will allocate an array to store all the
parent.
To calculate the amount of storage required for the parents
array, `alloc_parents` simply multiplicates the number of parents
with the respective elements's size. This already screams "buffer
overflow", and in fact this problem is getting worse by the
result being cast to an `uint32_t`.
In fact, triggering this is possible: git-hash-object(1) will
happily write a commit with multiple millions of parents for you.
I've stopped at 67,108,864 parents as git-hash-object(1)
unfortunately soaks up the complete object without streaming
anything to disk and thus will cause an OOM situation at a later
point. The point here is: this commit was about 4.1GB of size but
compressed down to 24MB and thus easy to distribute.
The above doesn't yet trigger the buffer overflow, thus. As the
array's elements are all pointers which are 8 bytes on 64 bit, we
need a total of 536,870,912 parents to trigger the overflow to
`0`. The effect is that we're now underallocating the array
and do an out-of-bound writes. As the buffer is kindly provided
by the adversary, this may easily result in code execution.
Extrapolating from the test file with 67m commits to the one with
536m commits results in a factor of 8. Thus the uncompressed
contents would be about 32GB in size and the compressed ones
192MB. While still easily distributable via the network, only
servers will have that amount of RAM and not cause an
out-of-memory condition previous to triggering the overflow. This
at least makes this attack not an easy vector for client-side use
of libgit2.
|
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
When the VirtualStore feature is in effect, it is safe to let random
users write into C:\ProgramData because other users won't see those
files. This seemed to be the case when we introduced support for
C:\ProgramData\Git\config.
However, when that feature is not in effect (which seems to be the case
in newer Windows 10 versions), we'd rather not use those files unless
they come from a trusted source, such as an administrator.
This change imitates the strategy chosen by PowerShell's native OpenSSH
port to Windows regarding host key files: if a system file is owned
neither by an administrator, a system account, or the current user, it
is ignored.
|
|\ \ \ \ \ \
| | | | | | |
| | | | | | | |
stash: avoid recomputing tree when committing worktree
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
When creating a new stash, we need to create there separate
commits storing differences stored in the index, untracked
changes as well as differences in the working directory. The
first two will only be done conditionally if the equivalent
options "git stash --keep-index --include-untracked" are being
passed to `git_stash_save`, but even when only creating a stash
of worktree changes we're much slower than git.git. Using our new
stash example:
$ time git stash
Saved working directory and index state WIP on (no branch): 2f7d9d47575e Linux 5.1.7
real 0m0.528s
user 0m0.309s
sys 0m0.381s
$ time lg2 stash
real 0m27.165s
user 0m13.645s
sys 0m6.403s
As can be seen, libgit2 is more than 50x slower than git.git!
When creating the stash commit that includes all worktree
changes, we create a completely new index to prepare for the new
commit and populate it with the entries contained in the index'
tree. Here comes the catch: by populating the index with a tree's
contents, we do not have any stat caches in the index. This means
that we have to re-validate every single file from the worktree
and see whether it has changed.
The issue can be fixed by populating the new index with the
repo's existing index instead of with the tree. This retains all
stat cache information, and thus we really only need to check
files that have changed stat information. This is semantically
equivalent to what we previously did: previously, we used the
tree of the commit computed from the index. Now we're just using
the index directly.
And, in fact, the cache is doing wonders:
time lg2 stash
real 0m1.836s
user 0m1.166s
sys 0m0.663s
We're now performing 15x faster than before and are only 3x
slower than git.git now.
|
|\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
Variadic macros
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
The macro `p_snprintf` is implemented as a variadic macro that
calls `snprintf` directly with `__VA_ARGS__`. In C89, variadic
macros are not allowed, but as the arguments of `p_snprintf` and
`snprintf` are matching 1:1, we can fix this by simply removing
the parameter list from `p_snprintf`.
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
The macro `apply_err` is implemented as a variadic macro, which
are not defined by C89. Convert it to a variadic function,
instead.
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
The macro `git_parse_error` is implemented in a variadic way so
that it's possible to pass printf-style parameters.
Unfortunately, variadic macros are not defined by C89 and thus we
cannot use that functionality. But as we have implemented
`git_error_vset` in the previous commit, we can now just use that
instead.
Convert `git_parse_error` to a variadic function and use
`git_error_vset` to fix the compliance violation. While at it,
move the function to "patch_parse.c".
|
| | |_|_|/ / /
| |/| | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Right now, we only provide a `git_error_set` that has a variadic
function signature. It's impossible to drive this function in a
C89-compliant way from other functions that have a variadic
signature, though, like for example `git_parse_error`.
Implement a new `git_error_vset` function that gets a `va_list`
as parameter, fixing the above problem.
|
|\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
Add sign capability to git_rebase_commit
|
| | | | | | | | |
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
This simplifies the flow of rebase_commit__create because it doesn't have to juggle 2 different commit flows (one with signature and one without).
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
If provided with a null signature, skip adding the signature header and create the commit anyway.
|
| | | | | | | | |
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
We should clear the error before calling the signing_cb to allow the signing_cb to set its own errors. If the CB did not provide an error, we should set our own generic error before exiting rebase_commit__create
|
| | | | | | | | |
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
In the case that we want to build merge + commit, cherrypick + commit, or even just build a commit with signing callback, `git_rebase_commit_signature_cb` particular callback should be made more generic. We also renamed `signature_cb` to `signing_cb` to improve clarity on the purpose of the callback (build a difference between a git_signature and the act of signing).
So we've ended up with `git_commit_signing_cb`.
|
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Reduces the number of callbacks for signing a commit during a rebase operation to just one callback. That callback has 2 out git_buf parameters for signature and signature field. We use git_buf here, because we cannot make any assumptions about the heap allocator a user of the library might be using.
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
2 callbacks have been added to git_rebase_options, git_rebase_commit_signature_cb and git_rebase_commit_signature_field_cb. When git_rebase_commit_signature_cb is present in git_rebase_options, it will be called whenever git_rebase_commit is performed, giving an opportunity to sign the commit. The signing procedure can be skipped if the callback specifies passthrough as the error. The git_rebase_commit_signature_field_cb will only be called if the other callback is present or did not passthrough, and it provides means to specify which field a signature is for.
Git_rebase_options was chosen as the home for these callbacks as it keeps backwards compatibility with the current rebase api.
|
|\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | | |
remote: remove unused block of code
|
| | |/ / / / / /
| |/| | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
In "remote.c", we have a chunk of code that is #ifdef'fed out via
`#if 0` with a comment that we could export it as a helper function.
The code was implemented in 2013 and ifdef'fed in 2014, which shows that
there's clearly no interest in having such a helper at all.
As this block has recently created some confusion about `p_getenv` due
to it containing the only reference to that function in our codebase,
let's remove this block altogether.
|
|\ \ \ \ \ \ \ \
| |/ / / / / / /
|/| | | | | | | |
config: check if we are running in a sandboxed environment
|
| | |_|/ / / /
| |/| | | | |
| | | | | | | |
On macOS the $HOME environment variable returns the path to the sandbox container instead of the actual user $HOME for sandboxed apps. To get the correct path, we have to get it from the password file entry.
|
|\ \ \ \ \ \ \
| |_|_|_|/ / /
|/| | | | | | |
config: separate file and snapshot backends
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The internal backend structures are kind-of legacy and do not really
speak for themselves. Rename them accordingly to make them easier to
understand.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
To further distinguish the file writeable and readonly backends,
separate the readonly backend into its own "config_snapshot.c"
implementation. The snapshot backend can be generically used to snapshot
any type of backend.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
In `backend_readonly_free`, the passed in config backend is being cast
to a `diskfile_backend` instead of to a `diskfile_readonly_backend`.
While this works out just fine because we only access its header values,
which were shared between both backends, it is undefined behaviour.
Use the correct type to fix this.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The `diskfile_header` structure is shared between both
`diskfile_backend` and `diskfile_readonly_backend`. The separation and
resulting casting is confusing at times and a source for programming
errors.
Remove the shared structure and inline them directly.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
While most functions of the readonly configuration backend are
implemented separately from the writeable configuration backend, the two
functions `config_iterator_new` and `config_get` are shared between
both. This sharing makes it necessary to have some shared data
structures, which is the `diskfile_header` structure. Unfortunately, this
makes the backends harder to grasp than necessary due to all the casting
between structs and also quite error prone.
Reimplement those functions for the readonly backends. As readonly
backends cannot be refreshed anyway, we can remove the calls to
`config_refresh` in there.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The `config_readonly_open` function currently receives as input a
diskfile backend and will copy its entries to a new snapshot. This is
rather intimate, as we need to assume that the source config backend is
in fact a diskfile entry. We can do better than this though by using
generic methods to copy contents of the provided backend, e.g. by using
a config iterator. This also allows us to decouple the read-only backend
from the read-write backend.
|
| |/ / / / /
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
When duplicating a configuration entry, we allocate a new entry but do
not verify that we get a valid pointer back. As we're dereferencing the
pointer afterwards, we might thus run into a segfault in out-of-memory
situations.
Extract a new function `git_config_entries_dup_entry` that handles the
complete entry duplication. Fix the error by using
`GIT_ERROR_CHECK_ALLOC`.
|
|/ / / / /
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
In #5118 we remove the double-underscore to make it a normally-named public
function. However, this is not an interesting function outside of the library
and it takes up a name for something that could be more useful.
Remove the single-underscore version as we have not done any releases with it.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
When creating a new iterator for a config file backend, then we should
always make sure that we're up to date by calling `config_refresh`.
Otherwise, we might not notice when another process has modified the
configuration file and thus will represent outdated values.
Add two tests to config::stress that verify that we get up-to-date
values when reading configuration entries via `git_config_iterator`.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
If calling `config_refresh` on a read-only configuration file backend,
then we will segfault when comparing the timestamp of the file due to
`path` being uninitialized. As a read-only snapshot should not be
refreshed anyway and stay consistent, we can simply return early when
calling `config_refresh` on a read-only snapshot.
|
| |_|/ /
|/| | | |
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
On most platforms it's fine to create symlinks to nonexisting files. Not
so on Windows, where the type of a symlink (file or directory) needs to
be set at creation time. So depending on whether the target file exists
or not, we may end up with different symlink types. This creates a
problem when performing checkouts, where we simply iterate over all blobs
that need to be updated without treating symlinks any special. If the
target file of the symlink is going to be checked out after the symlink
itself, then the symlink will be created as directory symlink and not as
file symlink.
Fix the issue by iterating over blobs twice: once to perform postponed
deletions and updates to non-symlink blobs, and once to perform updates
to symlink blobs.
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When creating a symlink in Windows, one needs to tell Windows whether
the symlink should be a file or directory symlink. To determine which
flag to pass, we call `GetFileAttributesW` on the target file to see
whether it is a directory and then pass the flag accordingly. The
problem though is if create a symlink with a relative target path, then
we will check that relative path while not necessarily being inside of
the working directory where the symlink is to be created. Thus, getting
its attributes will either fail or return attributes of the wrong
target.
Fix this by resolving the target path relative to the directory in which
the symlink is to be created.
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When deleting a symlink on Windows, then the way to delete it depends on
whether it is a directory symlink or a file symlink. In the first case,
we need to use `DeleteFile`, in the second `RemoveDirectory`. Right now,
`p_unlink` will only ever try to use `DeleteFile`, though, and thus fail
to remove directory symlinks. This mismatches how unlink(3P) is expected
to behave, though, as it shall remove any symlink disregarding whether
it is a file or directory symlink.
In order to correctly unlink a symlink, we thus need to check what kind
of file this is. If we were to first query file attributes of every file
upon calling `p_unlink`, then this would penalize the common case
though. Instead, we can try to first delete the file with `DeleteFile`
and only if the error returned is `ERROR_ACCESS_DENIED` will we query
file attributes and determine whether it is a directory symlink to use
`RemoveDirectory` instead.
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When initializing a repository, we need to check whether its working
directory supports symlinks to correctly set the initial value of the
"core.symlinks" config variable. The code to check the filesystem is
reusable in other parts of our codebase, like for example in our tests
to determine whether certain tests can be expected to succeed or not.
Extract the code into a new function `git_path_supports_symlinks` to
avoid duplicate implementations. Remove a duplicate implementation in
the repo test helper code.
|
| |/ /
|/| |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Our file utils functions all have a "futils" prefix, e.g.
`git_futils_touch`. One would thus naturally guess that their
definitions and implementation would live in files "futils.h" and
"futils.c", respectively, but in fact they live in "fileops.h".
Rename the files to match expectations.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
With commit dedf70ad2 (patch_parse: do not depend on parsed buffer's
lifetime, 2019-07-05), all lines of the patch are allocated with
`strdup` to make lifetime of the parsed patch independent of the buffer
that is currently being parsed. In patch b08932824 (patch_parse: ensure
valid patch output with EOFNL, 2019-07-11), we introduced another
code location where we add lines to the parsed patch. But as that one
was implemented via a separate pull request, it wasn't converted to use
`strdup`, as well. As a consequence, we generate a segfault when trying
to deallocate the potentially static buffer that's now in some of the
lines.
Use `git__strdup` to fix the issue.
|
|\ \ \
| | | |
| | | | |
ignore: fix determining whether a shorter pattern negates another
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When computing whether we need to store a negative pattern, we iterate
through all previously known patterns and check whether the negative
pattern undoes any of the previous ones. In doing so we call `wildmatch`
and check it's return for any negative error values. If there was a
negative return, we will abort and bubble up that error to the caller.
In fact, this check for negative values stems from the time where we
still used `fnmatch` instead of `wildmatch`. For `fnmatch`, negative
values indicate a "real" error, while for `wildmatch` a negative value
may be returned if the matching was prematurely aborted. A premature
abort may for example also happen if the pattern matches a prefix of the
haystack if the pattern is shorter. Returning an error in that case is
the wrong thing to do.
Fix the code to compare for equality with `WM_MATCH`, only. Negative
values returned by `wildmatch` are perfectly fine and thus should be
ignored. Add a test that verifies we do not see the error.
|