Zip Slip [1] is an arbitrary file write generic vulnerability, that can
be achieved using a specially crafted zip (or bzip2, gzip, tar, xz, war)
archive, that holds path traversal filenames.
According to Maven's announcement [2] several plugins use plexus-archiver to
unpack dependencies to disk and have been identified as potential triggers
for exposing the vulnerability.
Of those, JGit uses the maven-dependency-plugin and the maven-javadoc-plugin.
Update them to the fixed versions reported in [2].
See the corresponding issues for the maven-dependency-plugin [3] and the
maven-javadoc-plugin [4] for details.
[1] https://snyk.io/research/zip-slip-vulnerability
[2] https://maven.apache.org/security-plexus-archiver.html
[3] https://issues.apache.org/jira/browse/MDEP-611
[4] https://issues.apache.org/jira/browse/MJAVADOC-520
Change-Id: Id3ab2d6161db240f2ab8f82298fa3ecd7a930a43
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Ensure that notifyIndexChanged is called every time we call
FileSnapshot.save, except the first.
Change-Id: I5a4e9826e791f518787366ae7c3a0ef3d416d2c1
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-5.0:
Teach UploadPack "filter" in protocol v2 fetch
Refactor test of capabilities output
Refactor v2 advertisement into own function
Refactor parsing of "filter" into its own method
Disallow unknown args to "fetch" in protocol v2
Teach UploadPack shallow fetch in protocol v2
Refactor unshallowCommits to local variable
Add protocol v2 support in http
Give info/refs services more control over response
Change-Id: I1683902222e076e1091795e94790a264550afb7b
Signed-off-by: Jonathan Nieder <jrn@google.com>
If the configuration variable uploadpack.allowfilter is true, advertise
that "filter" is supported, and support it if the client sends such an
argument.
Change-Id: I7de66c0a0ada46ff71c5ba124d4ffa7c47254c3b
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
A subsequent patch will dynamically generate the capability
advertisement, so the capability advertisements produced are not always
the same. Separate the checking of the advertisements into its own test
method.
Change-Id: I768d14b9d1a244d5d886c42ffd62ef3957b518fb
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
A subsequent patch needs dynamic generation of this advertisement
depending on a configuration variable in the underlying repository, so
refactor it into a function instead of using a constant list.
Change-Id: Ie00584add1fb56c9e88c7b57f75703981ea5bb85
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
The implementation of protocol v2 will also need to parse the "filter"
option, so refactor it into its own method.
Change-Id: I751f6e6ca63fab873298594653a3885202297a2e
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
JGit's implementation of the fetch command of protocol v2, unlike its
implementation of ls-refs, currently tolerates unknown arguments.
Tighten fetch to not allow unrecognized arguments and add tests to
verify this behavior for both ls-refs and fetch.
Change-Id: I321161d568bd638252fab1a47b06b924d472a669
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "shallow" and "deepen" parameters in the "fetch"
command in the fetch-pack/upload-pack protocol v2. Advertise support for
this in the capability advertisement.
TODO: implement deepen-relative, deepen-since, deepen-not
Change-Id: I7ffd80d6c38872f9d713ac7d6e0412106b3766d7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
This reduces the amount of state held as instance variables in
UploadPack, and makes it easier for a future patch to contain a clearer
version of UploadPack#processShallow.
Change-Id: I6df80b42f9e5118fda1420692e02e417670cced3
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Teach UploadPack to support protocol v2 with non-bidirectional pipes,
and add support to the HTTP protocol for v2. This is only activated if
the repository's config has "protocol.version" equal to 2.
Change-Id: I093a14acd2c3850b8b98e14936a716958f35a848
Helped-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Currently, SmartServiceInfoRefs always prints "# service=serviceName"
followed by a flush packet in response to an info/refs request, and then
hands it off to the specific service class. Printing of "#
service=serviceName" is mandated for protocol v0, but not v2.
Therefore, the existing code works for protocol v0, but whenever a
service that supports protocol v2 receives an info/refs request, it must
first determine which protocol version is to be used (depending on, for
example, the request and any relevant configuration variables), and then
decide if "# service=serviceName" needs to be printed.
Create a new method that v2-supporting service classes can override,
covering the printing of both "# service=serviceName" and everything
that the #advertise method prints. This will be used in a subsequent
commit in which UploadPackServlet (and the other classes it uses) is
updated to support protocol v2.
Change-Id: Ia026b06e96a6b15937514096babd024ef77df1ea
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Provide a factory for comparators that use the default heuristics except
with a different ordering of PackSources.
Change-Id: I0809b64deb3d0486040076946fdbdad650d69240
There are several ways of comparing DfsPackDescriptions for different
purposes, such as object lookup search order and reftable ordering. Some
of these are later compounded into comparators on other objects, so they
appear in the code as Comparator<DfsReftable>, for example.
Put all the DfsPackDescription comparators in static methods on
DfsPackDescription itself. Stop implementing Comparable, to avoid giving
the impression that there is always one true and correct way of sorting
packs.
Change-Id: Ia5ca65249c13373f7ef5b8a5d1ad50a26577706c
Rather than requiring callers to do their own computations based on the
package-private "category" number, provide an actual
Comparator<PackSource> instance, and explicitly discourage usage of
default Enum comparison.
Construct the default comparator using a builder pattern based on
defining equivalence classes. This gives us the same behavior as the old
category field in PackSource, with an abstraction that does not leak the
implementation detail of comparing rank numbers.
Change-Id: I6757211397ab1bc181d61298e073f88b69dbefc3
In normal operation, the source of a pack should never be null; the DFS
implementation should always know where a pack came from. Existing
implementations in InMemoryRepository and at Google always have the
source available at construction time.
The problem with null PackSources in the previous implementation was it
made the DfsPackDescription#compareTo method intransitive. Specifically,
it skips comparing the sources at all if *either* operand is null.
Suppose we have three descriptions A, B, and C, where all fields are
equal except the PackSource, and:
* A's source is INSERT
* B's source is null
* C's source is RECEIVE
In this case, A.compareTo(B) == 0, and B.compareTo(C) == 0, since all
fields are equal except the source, which is skipped. But
A.compareTo(C) != 0, since A and B have different sources.
Avoid this problem in compareTo by enforcing that the source is never
null. We could of course assign an arbitrary category number to a null
source in order to make comparison transitive[1], but it's simpler to
implement and reason about if the field is non-nullable, and there is no
real-world use case to make it null.
Although a non-null source is required at construction time, the field
is currently still mutable: DfsPackDecscription#setPackSource is used by
DfsInserterTest to mark packs as garbage. This could probably be
avoided as well, allowing us to convert packSource to a final field, but
doing so is beyond the scope of this change.
[1] The astute reader will notice this is already done by
DfsObjDatabase#reftableComparator(). In fact, the reason that
different comparator implementations non-obviously have different
semantics for this nullable field is another reason why it's clearer
to avoid null entirely.
Change-Id: I85a2aaf3fd6d4868f241f7972a0349f087830ffa
Tests that even a commit with a huge message can be committed and read
if the WindowCache's streamFileThreshold is configured large enough.
Bug: 535092
Change-Id: Id8090c608625010caf11dff7971b47882b5fd20f
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
* stable-5.0:
Fix description property in feature.properties of pgm source feature
Update SUA
Change-Id: I891a7b8b922ecae49872eb642d6917e8c85c2f51
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The canonical implementation also doesn't. Compare current
code in remote.c, function get_stale_heads_cb.[1] Not handling
symrefs in this case was introduced in canonical git in [2]
in 2008.
[1] https://github.com/git/git/blob/v2.17.0/remote.c#L2259
[2] https://github.com/git/git/commit/740fdd27f0
Bug: 533549
Change-Id: If348d56bb4a96b8aa7141f7e7b5a0d3dd4e7808b
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Callers of getAllRefs that only iterate over the `values()` of the
returned map can be trivially fixed to call getRefDatabase().getRefs()
instead.
Only fix those where the calling method is already declared to throw
IOException, to avoid potential API changes.
Change-Id: I2b05f785077a1713953cfd42df7bf915f889f90b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Callers should instead use getRefDatabase().getRefs(), which does not
swallow the IOException.
Replace @link with @code in the Javadoc of FileRepository, since linking
to the deprecated method causes an error:
Javadoc: The method getAllRefs() from the type Repository is deprecated
Existing callers of the deprecated method are not adapted in this commit
because many of them require more refactoring. They will be done in
separate follow-up commits.
Bug: 534731
Change-Id: Id84e70e4cd7be3d1ca1795512950c6abe3d18ffd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* master:
DescribeCommand: Refactor to not use deprecated Repository#peel
Repository: Deprecate #peel method
Repository: Make #exactRef and #findRef final
Skip ignored directories in FileTreeIterator
Repository: Deprecate getTags method
InfoRefsServlet: Refactor to not use deprecated methods
RefAdvertiser: Add send(Collection<Ref>) and deprecate send(Map<String, Ref>)
Remove deprecated Repository#notifyIndexChanged
Implementors should override Repository#notifyIndexChanged(boolean)
Revive Repository#notifyIndexChanged()
Remove further unnecessary 'final' keywords
Execute AdvertiseRefsHook only for protocol v0 and v1
Add protocol v2 support in "jgit daemon"
Teach UploadPack "ofs-delta" in "fetch"
Teach UploadPack "include-tag" in "fetch"
Avoid using #refs in UploadPack#sendPack
FileRepository: Don't use deprecated RefDatabase#getRefs(String)
BatchRefUpdate: Don't use deprecated RefDatabase#getRefs(String)
Change-Id: I16c5da62d09262d3f4070aa0f466dd6c8352b5ea
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Callers should use getRefDatabase().peel(ref) instead since it
doesn't swallow the IOException.
Adapt all trivial callers to user the alternative.
DescribeCommand still uses the deprecated method and is not adapted in
this change since it will require more refactoring to add handling of
the IOException.
Change-Id: I14d4a95a5e0570548753b9fc5c03d024dc3ff832
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This means less cognitive overhead for both implementors and callers,
since this way we can guarantee that they are always synonyms for
RefDatabase#exactRef and RefDatabase#findRef, respectively.
Change-Id: Ic8aeb52fc7ed65672f3f6cd1da39a66908d88baa
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Make FileTreeIterator not enter ignored directories by default. We
only need to enter ignored directories if we do some operation against
git, and there is at least one tracked file underneath an ignored
directory.
Walking ignored directories should be avoided as much as possible as
it is a potential performance bottleneck. Some projects have a lot of
files or very deep hierarchies in ignored directories; walking those
may be costly (especially so on Windows). See for instance also bug
500106.
Provide a FileTreeIterator.setWalkIgnoredDirectories() operation to
force the iterator to iterate also through otherwise ignored
directories. Useful for tests (IgnoreNodeTest, CGitIgnoreTest), or
to implement things like "git ls-files --ignored".
Add tests in DirCacheCheckoutTest, and amend IndexDiffTest to test a
little bit more.
Bug: 388582
Change-Id: I6ff584a42c55a07120a4369fd308409431bdb94a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Callers should use getRefDatabase().getRefsByPrefix(R_TAGS) instead.
Adjust the tests accordingly.
Bug: 534731
Change-Id: Ib28ae365e42720268996ff46e34cae1745ad545c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Declare Repository#notifyIndexChanged() final and
Repository#notifyIndexChanged(boolean) abstract to force implementors
to switch to overriding the latter method. This makes Repository less
error-prone to extend since implementors no longer need to remember to
override one of those two methods.
Change-Id: I721db0f4a4865db3b35212ee0a2045d5b31c96af
Signed-off-by: Jonathan Nieder <jrn@google.com>
e9e150fdd2 (Store in IndexChangedEvent if it was caused by JGit
itself, 2018-05-13) modified Repository#notifyIndexChanged to take a
boolean argument to indicate whether the index change happened under
the current process's control or externally, for use by EGit. In
other words, the function signature changed from
public abstract void notifyIndexChanged();
to
public abstract void notifyIndexChanged(boolean internal);
Callers outside JGit itself notifying a Repository about index changes
are expected to be rare, so this is not very disruptive to them. In
most cases they would be notifying about changes that they made
themselves, so treating their notifyIndexChanged() calls as
notifyIndexChanged(true) should be relatively safe.
Implementors have the opposite problem: adding the new "abstract void
notifyIndexChanged(boolean)" method means they are obligated to
override it. Add a default implementation that calls their existing
override of notifyIndexChanged() to make their migration easier.
The main downside is that authors of new Repository subclasses that
do not realize they need to override notifyIndexChanged would end up
with a default implementation which calls notifyIndexChanged(true),
in turn calling notifyIndexChanged() again and so on, resulting in
StackOverflowException. Add an implementors' note to the class
Javadoc to avoid this issue. A followup commit will force
implementors to adapt to the new API by changing the methods to
@Deprecated
public final void notifyIndexChanged() {
notifyIndexChanged(true);
}
public abstract void notifyIndexChanged(boolean internal);
Change-Id: I7d014890ee19abf283ea824d9baa9044bfdde130
Signed-off-by: Jonathan Nieder <jrn@google.com>
Matthias Sohn
David Pursehouse
Jonathan Nieder
Jonathan Tan
Mincong HUANG
Dave Borowitz
Thomas Wolf