Skip to content

Commit d2f8c04

Browse files
author
Vladimir Kotal
authored
disallow repository path equal to source root (#2801)
1 parent d4c87e0 commit d2f8c04

File tree

4 files changed

+54
-14
lines changed

4 files changed

+54
-14
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,9 @@ private Collection<RepositoryInfo> addRepositories(File[] files,
414414
} catch (IllegalAccessException iae) {
415415
LOGGER.log(Level.WARNING, "Could not create repository for '"
416416
+ file + "', missing access rights.", iae);
417+
} catch (ForbiddenSymlinkException e) {
418+
LOGGER.log(Level.WARNING, "Could not create repository for '"
419+
+ file + "', path traversal issues.", e);
417420
}
418421
if (repository == null) {
419422
// Not a repository, search its sub-dirs.

opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryFactory.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import org.opengrok.indexer.configuration.RuntimeEnvironment;
3535
import org.opengrok.indexer.logger.LoggerFactory;
36+
import org.opengrok.indexer.util.ForbiddenSymlinkException;
3637

3738
/**
3839
* This is a factory class for the different repositories.
@@ -80,7 +81,7 @@ public static List<Class<? extends Repository>> getRepositoryClasses() {
8081
}
8182

8283
public static Repository getRepository(File file)
83-
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException{
84+
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException, IOException, ForbiddenSymlinkException {
8485
return getRepository(file, false);
8586
}
8687

@@ -101,15 +102,25 @@ public static Repository getRepository(File file)
101102
* @throws IllegalAccessException in case no permissions to repository file
102103
* @throws NoSuchMethodException in case we cannot create the repository object
103104
* @throws InvocationTargetException in case we cannot create the repository object
105+
* @throws IOException when resolving repository path
106+
* @throws ForbiddenSymlinkException when resolving repository path
104107
*/
105108
public static Repository getRepository(File file, boolean interactive)
106-
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
109+
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException, IOException, ForbiddenSymlinkException {
107110
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
108111
Repository repo = null;
109112

110113
for (Repository rep : repositories) {
111114
if (rep.isRepositoryFor(file, interactive)) {
112115
repo = rep.getClass().getDeclaredConstructor().newInstance();
116+
117+
if (env.isProjectsEnabled() && env.getPathRelativeToSourceRoot(file).equals(File.separator)) {
118+
LOGGER.log(Level.WARNING, "{0} was detected as {1} repository however with directory " +
119+
"matching source root. This is invalid because projects are enabled. Ignoring this " +
120+
"repository.",
121+
new Object[]{file, rep.getType()});
122+
return null;
123+
}
113124
repo.setDirectoryName(file);
114125

115126
if (!repo.isWorking()) {
@@ -181,9 +192,11 @@ public static Repository getRepository(File file, boolean interactive)
181192
* @throws IllegalAccessException in case no permissions to repository
182193
* @throws NoSuchMethodException in case we cannot create the repository object
183194
* @throws InvocationTargetException in case we cannot create the repository object
195+
* @throws IOException when resolving repository path
196+
* @throws ForbiddenSymlinkException when resolving repository path
184197
*/
185198
public static Repository getRepository(RepositoryInfo info, boolean interactive)
186-
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
199+
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException, IOException, ForbiddenSymlinkException {
187200
return getRepository(new File(info.getDirectoryName()), interactive);
188201
}
189202

opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryFactoryTest.java

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,18 @@
2323
package org.opengrok.indexer.history;
2424

2525
import java.io.File;
26+
import java.io.IOException;
2627
import java.lang.reflect.InvocationTargetException;
2728
import org.junit.AfterClass;
2829
import static org.junit.Assert.assertFalse;
30+
import static org.junit.Assert.assertNull;
31+
2932
import org.junit.BeforeClass;
3033
import org.junit.Test;
34+
import org.opengrok.indexer.condition.ConditionalRun;
35+
import org.opengrok.indexer.condition.RepositoryInstalled;
36+
import org.opengrok.indexer.configuration.RuntimeEnvironment;
37+
import org.opengrok.indexer.util.ForbiddenSymlinkException;
3138
import org.opengrok.indexer.util.TestRepository;
3239

3340
/**
@@ -51,16 +58,31 @@ public static void tearDown() {
5158
repository = null;
5259
}
5360
}
54-
61+
62+
@ConditionalRun(RepositoryInstalled.MercurialInstalled.class)
63+
@Test
64+
public void testRepositoryMatchingSourceRoot() throws IllegalAccessException, InvocationTargetException,
65+
ForbiddenSymlinkException, InstantiationException, NoSuchMethodException, IOException {
66+
67+
File root = new File(repository.getSourceRoot(), "mercurial");
68+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
69+
env.setSourceRoot(root.getAbsolutePath());
70+
env.setProjectsEnabled(true);
71+
assertNull(RepositoryFactory.getRepository(root));
72+
}
73+
5574
/*
56-
* There is no conditonal run based on whether given repository is installed because
75+
* There is no conditional run based on whether given repository is installed because
5776
* this test is not supposed to have working Mercurial anyway.
5877
*/
59-
private void testNotWorkingRepository(String repoPath, String propName)
60-
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
61-
78+
private void testNotWorkingRepository(String repoPath, String propName)
79+
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException,
80+
IOException, ForbiddenSymlinkException {
81+
82+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
6283
String origPropValue = System.setProperty(propName, "/foo/bar/nonexistent");
6384
File root = new File(repository.getSourceRoot(), repoPath);
85+
env.setSourceRoot(repository.getSourceRoot());
6486
Repository repo = RepositoryFactory.getRepository(root);
6587
if (origPropValue != null) {
6688
System.setProperty(propName, origPropValue);
@@ -69,14 +91,16 @@ private void testNotWorkingRepository(String repoPath, String propName)
6991
}
7092

7193
@Test
72-
public void testNotWorkingMercurialRepository()
73-
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
94+
public void testNotWorkingMercurialRepository()
95+
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException,
96+
IOException, ForbiddenSymlinkException {
7497
testNotWorkingRepository("mercurial", MercurialRepository.CMD_PROPERTY_KEY);
7598
}
7699

77100
@Test
78-
public void testNotWorkingBitkeeperRepository()
79-
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
101+
public void testNotWorkingBitkeeperRepository()
102+
throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException,
103+
IOException, ForbiddenSymlinkException {
80104
testNotWorkingRepository("bitkeeper", BitKeeperRepository.CMD_PROPERTY_KEY);
81105
}
82106
}

opengrok-indexer/src/test/java/org/opengrok/indexer/util/TestRepository.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ public void removeDummyFile(String project) {
126126
* @param filename a required instance
127127
* @param in a required instance
128128
* @param project an optional project name
129-
* @return
130-
* @throws IOException
129+
* @return file object
130+
* @throws IOException I/O exception
131131
*/
132132
public File addAdhocFile(String filename, InputStream in, String project)
133133
throws IOException {

0 commit comments

Comments
 (0)