This is an automated email from the ASF dual-hosted git repository.
janhoy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new c1f916d SOLR-15826: ResourceLoader should better respect allowed
paths (#439)
c1f916d is described below
commit c1f916d532e9580f22c09b6ad746d4ea9bcc47a6
Author: Jan Høydahl <[email protected]>
AuthorDate: Fri Dec 3 23:39:25 2021 +0100
SOLR-15826: ResourceLoader should better respect allowed paths (#439)
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/core/SolrResourceLoader.java | 58 ++++++++++++----------
.../solr/schema/ManagedIndexSchemaFactory.java | 2 +-
.../org/apache/solr/core/ResourceLoaderTest.java | 41 +++++++++++++--
.../apache/solr/schema/PrimitiveFieldTypeTest.java | 5 +-
.../org/apache/solr/util/TestSystemIdResolver.java | 4 +-
6 files changed, 76 insertions(+), 36 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c6c68d8..9b04022 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -461,6 +461,8 @@ Bug Fixes
* SOLR-15825: Security UI 'hasPermission' check should check if the user has
the "all" permission if the requested permission is not defined
to match how the backend works (Timothy Potter)
+* SOLR-15826: ResourceLoader should better respect allowed paths (janhoy)
+
* SOLR-15828: AuthTool (in SolrCLI) should include the config-read,
collection-admin-read, core-admin-read, and all permissions in the initial
security.json
to avoid warnings in the security UI (Timothy Potter)
diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
index 83e15a2..bac4709 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -90,6 +90,8 @@ public class SolrResourceLoader implements ResourceLoader,
Closeable, SolrClassL
"spelling.suggest.", "spelling.suggest.fst.", "rest.schema.analysis.",
"security.", "handler.admin."
};
private static final Charset UTF_8 = StandardCharsets.UTF_8;
+ public static final String SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM =
"solr.allow.unsafe.resourceloading";
+ private boolean allowUnsafeResourceloading;
private String name = "";
@@ -164,7 +166,8 @@ public class SolrResourceLoader implements ResourceLoader,
Closeable, SolrClassL
* found in the "lib/" directory in the specified instance directory.
*/
public SolrResourceLoader(Path instanceDir, ClassLoader parent) {
- if (instanceDir == null) {
+ allowUnsafeResourceloading =
Boolean.getBoolean(SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM);
+ if (instanceDir == null) {
throw new NullPointerException("SolrResourceLoader instanceDir must be
non-null");
}
@@ -320,16 +323,6 @@ public class SolrResourceLoader implements ResourceLoader,
Closeable, SolrClassL
return classLoader;
}
- private Path checkPathIsSafe(Path pathToCheck) throws IOException {
- if (Boolean.getBoolean("solr.allow.unsafe.resourceloading"))
- return pathToCheck;
- pathToCheck = pathToCheck.normalize();
- if (pathToCheck.startsWith(instanceDir))
- return pathToCheck;
- throw new IOException("File " + pathToCheck + " is outside resource loader
dir " + instanceDir +
- "; set -Dsolr.allow.unsafe.resourceloading=true to allow unsafe
loading");
- }
-
/**
* Opens any resource by its name.
* By default, this will look in multiple locations to load the resource:
@@ -342,15 +335,21 @@ public class SolrResourceLoader implements
ResourceLoader, Closeable, SolrClassL
*/
@Override
public InputStream openResource(String resource) throws IOException {
+ if (resource.trim().startsWith("\\\\")) { // Always disallow UNC paths
+ throw new SolrResourceNotFoundException("Resource '" + resource + "'
could not be loaded.");
+ }
+ Path instanceDir = getInstancePath().normalize();
+ Path inInstanceDir = getInstancePath().resolve(resource).normalize();
+ Path inConfigDir =
instanceDir.resolve("conf").resolve(resource).normalize();
+ if (inInstanceDir.startsWith(instanceDir) || allowUnsafeResourceloading) {
+ // The resource is either inside instance dir or we allow unsafe
loading, so allow testing if file exists
+ if (Files.exists(inConfigDir) && Files.isReadable(inConfigDir)) {
+ return Files.newInputStream(inConfigDir);
+ }
- Path inConfigDir = getInstancePath().resolve("conf").resolve(resource);
- if (Files.exists(inConfigDir) && Files.isReadable(inConfigDir)) {
- return Files.newInputStream(checkPathIsSafe(inConfigDir));
- }
-
- Path inInstanceDir = getInstancePath().resolve(resource);
- if (Files.exists(inInstanceDir) && Files.isReadable(inInstanceDir)) {
- return Files.newInputStream(checkPathIsSafe(inInstanceDir));
+ if (Files.exists(inInstanceDir) && Files.isReadable(inInstanceDir)) {
+ return Files.newInputStream(inInstanceDir);
+ }
}
// Delegate to the class loader (looking into $INSTANCE_DIR/lib jars).
@@ -373,13 +372,20 @@ public class SolrResourceLoader implements
ResourceLoader, Closeable, SolrClassL
* Report the location of a resource found by the resource loader
*/
public String resourceLocation(String resource) {
- Path inConfigDir = getInstancePath().resolve("conf").resolve(resource);
- if (Files.exists(inConfigDir) && Files.isReadable(inConfigDir))
- return inConfigDir.normalize().toString();
+ if (resource.trim().startsWith("\\\\")) {
+ // Disallow UNC
+ return null;
+ }
+ Path inInstanceDir = instanceDir.resolve(resource).normalize();
+ Path inConfigDir =
instanceDir.resolve("conf").resolve(resource).normalize();
+ boolean isRelativeToInstanceDir =
inInstanceDir.startsWith(instanceDir.normalize());
+ if (isRelativeToInstanceDir || allowUnsafeResourceloading) {
+ if (Files.exists(inConfigDir) && Files.isReadable(inConfigDir))
+ return inConfigDir.normalize().toString();
- Path inInstanceDir = getInstancePath().resolve(resource);
- if (Files.exists(inInstanceDir) && Files.isReadable(inInstanceDir))
- return inInstanceDir.normalize().toString();
+ if (Files.exists(inInstanceDir) && Files.isReadable(inInstanceDir))
+ return inInstanceDir.normalize().toString();
+ }
try (InputStream is =
classLoader.getResourceAsStream(resource.replace(File.separatorChar, '/'))) {
if (is != null)
@@ -388,7 +394,7 @@ public class SolrResourceLoader implements ResourceLoader,
Closeable, SolrClassL
// ignore
}
- return resource;
+ return allowUnsafeResourceloading ? resource : null;
}
/**
diff --git
a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
index 7c3e670..f027486 100644
--- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
+++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
@@ -383,7 +383,7 @@ public class ManagedIndexSchemaFactory extends
IndexSchemaFactory implements Sol
*/
private File locateConfigFile(String resource) {
String location = config.getResourceLoader().resourceLocation(resource);
- if (location.equals(resource) || location.startsWith("classpath:"))
+ if (location == null || location.equals(resource) ||
location.startsWith("classpath:"))
return null;
return new File(location);
}
diff --git a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
index de9bd39..d37f749 100644
--- a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
+++ b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
@@ -42,12 +42,18 @@ import org.apache.solr.handler.admin.LukeRequestHandler;
import org.apache.solr.handler.component.FacetComponent;
import org.apache.solr.response.JSONResponseWriter;
import org.apache.solr.util.plugin.SolrCoreAware;
+import org.junit.After;
-import static org.apache.solr.core.SolrResourceLoader.assertAwareCompatibility;
-import static org.apache.solr.core.SolrResourceLoader.clearCache;
+import static org.apache.solr.core.SolrResourceLoader.*;
import static org.hamcrest.core.Is.is;
public class ResourceLoaderTest extends SolrTestCaseJ4 {
+ @Override
+ @After
+ public void tearDown() throws Exception {
+ super.tearDown();
+ setUnsafeResourceLoading(false);
+ }
public void testInstanceDir() throws Exception {
final Path dir = createTempDir();
@@ -63,13 +69,38 @@ public class ResourceLoaderTest extends SolrTestCaseJ4 {
Path instanceDir = temp.resolve("instance");
Files.createDirectories(instanceDir.resolve("conf"));
+ setUnsafeResourceLoading(false);
+ try (SolrResourceLoader loader = new SolrResourceLoader(instanceDir)) {
+ // Path traversal
+ assertTrue(assertThrows(IOException.class, () ->
+
loader.openResource("../../dummy.txt").close()).getMessage().contains("Can't
find resource"));
+ assertNull(loader.resourceLocation("../../dummy.txt"));
+
+ // UNC paths
+ assertTrue(assertThrows(SolrResourceNotFoundException.class, () ->
+
loader.openResource("\\\\192.168.10.10\\foo").close()).getMessage().contains("Resource
'\\\\192.168.10.10\\foo' could not be loaded."));
+ assertNull(loader.resourceLocation("\\\\192.168.10.10\\foo"));
+ }
+
+ setUnsafeResourceLoading(true);
try (SolrResourceLoader loader = new SolrResourceLoader(instanceDir)) {
+ // Path traversal - unsafe but allowed
loader.openResource("../../dummy.txt").close();
- fail();
- } catch (IOException ioe) {
- assertTrue(ioe.getMessage().contains("is outside resource loader dir"));
+ assertNotNull(loader.resourceLocation("../../dummy.txt"));
+
+ // UNC paths never allowed
+ assertTrue(assertThrows(SolrResourceNotFoundException.class, () ->
+
loader.openResource("\\\\192.168.10.10\\foo").close()).getMessage().contains("Resource
'\\\\192.168.10.10\\foo' could not be loaded."));
+ assertNull(loader.resourceLocation("\\\\192.168.10.10\\foo"));
}
+ }
+ private void setUnsafeResourceLoading(boolean unsafe) {
+ if (unsafe) {
+ System.setProperty(SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM, "true");
+ } else {
+ System.clearProperty(SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM);
+ }
}
@SuppressWarnings({"unchecked"})
diff --git
a/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
b/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
index 4b9b2dc..3dd53c6 100644
--- a/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
@@ -24,6 +24,7 @@ import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.core.SolrConfig;
+import org.apache.solr.core.SolrResourceLoader;
import org.junit.Test;
/**
@@ -42,7 +43,7 @@ public class PrimitiveFieldTypeTest extends SolrTestCaseJ4 {
System.setProperty("enable.update.log", "false"); // schema12 doesn't
support _version_
System.setProperty("solr.test.sys.prop1", "propone");
System.setProperty("solr.test.sys.prop2", "proptwo");
- System.setProperty("solr.allow.unsafe.resourceloading", "true");
+
System.setProperty(SolrResourceLoader.SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM,
"true");
initMap = new HashMap<>();
config = new SolrConfig(TEST_PATH().resolve("collection1"), testConfHome +
"solrconfig.xml");
@@ -50,7 +51,7 @@ public class PrimitiveFieldTypeTest extends SolrTestCaseJ4 {
@Override
public void tearDown() throws Exception {
- System.clearProperty("solr.allow.unsafe.resourceloading");
+
System.clearProperty(SolrResourceLoader.SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM);
super.tearDown();
}
diff --git a/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
b/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
index 18a7f70..a64f169 100644
--- a/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
+++ b/solr/core/src/test/org/apache/solr/util/TestSystemIdResolver.java
@@ -30,7 +30,7 @@ import org.xml.sax.InputSource;
public class TestSystemIdResolver extends SolrTestCaseJ4 {
public void tearDown() throws Exception {
- System.clearProperty("solr.allow.unsafe.resourceloading");
+
System.clearProperty(SolrResourceLoader.SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM);
super.tearDown();
}
@@ -93,7 +93,7 @@ public class TestSystemIdResolver extends SolrTestCaseJ4 {
}
public void testUnsafeResolving() throws Exception {
- System.setProperty("solr.allow.unsafe.resourceloading", "true");
+
System.setProperty(SolrResourceLoader.SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM,
"true");
final Path testHome =
SolrTestCaseJ4.getFile("solr/collection1").getParentFile().toPath();
final ResourceLoader loader = new
SolrResourceLoader(testHome.resolve("collection1"),
this.getClass().getClassLoader());
