You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ja...@apache.org on 2021/12/06 13:49:37 UTC
[lucene-solr] branch branch_8_11 updated: SOLR-15826 ResourceLoader should better respect allowed paths (#2622)
This is an automated email from the ASF dual-hosted git repository.
janhoy pushed a commit to branch branch_8_11
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8_11 by this push:
new aa95d54 SOLR-15826 ResourceLoader should better respect allowed paths (#2622)
aa95d54 is described below
commit aa95d5440d3eef11fe20ffddd34ef7385cbefbb2
Author: Jan Høydahl <ja...@users.noreply.github.com>
AuthorDate: Mon Dec 6 14:49:18 2021 +0100
SOLR-15826 ResourceLoader should better respect allowed paths (#2622)
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/core/SolrResourceLoader.java | 85 ++++++++++------------
.../solr/schema/ManagedIndexSchemaFactory.java | 2 +-
.../org/apache/solr/core/ResourceLoaderTest.java | 44 +++++++++--
.../apache/solr/schema/PrimitiveFieldTypeTest.java | 2 -
5 files changed, 81 insertions(+), 54 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a1e781a..9c81dfb 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -30,6 +30,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 b58c8d9..e50b58c 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -76,7 +76,8 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
"cloud.autoscaling."
};
private static final Charset UTF_8 = StandardCharsets.UTF_8;
-
+ public static final String SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM = "solr.allow.unsafe.resourceloading";
+ private final boolean allowUnsafeResourceloading;
private String name = "";
protected URLClassLoader classLoader;
@@ -89,9 +90,9 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
private PackageListeningClassLoader schemaLoader ;
private PackageListeningClassLoader coreReloadingClassLoader ;
- private final List<SolrCoreAware> waitingForCore = Collections.synchronizedList(new ArrayList<SolrCoreAware>());
- private final List<SolrInfoBean> infoMBeans = Collections.synchronizedList(new ArrayList<SolrInfoBean>());
- private final List<ResourceLoaderAware> waitingForResources = Collections.synchronizedList(new ArrayList<ResourceLoaderAware>());
+ private final List<SolrCoreAware> waitingForCore = Collections.synchronizedList(new ArrayList<>());
+ private final List<SolrInfoBean> infoMBeans = Collections.synchronizedList(new ArrayList<>());
+ private final List<ResourceLoaderAware> waitingForResources = Collections.synchronizedList(new ArrayList<>());
private final Properties coreProperties; // gone in 9.0
@@ -166,6 +167,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
@Deprecated // the Properties arg in particular is what is deprecated
public SolrResourceLoader(Path instanceDir, ClassLoader parent, Properties properties) {
this.coreProperties = properties;
+ allowUnsafeResourceloading = Boolean.getBoolean(SOLR_ALLOW_UNSAFE_RESOURCELOADING_PARAM);
if (instanceDir == null) {
this.instanceDir = SolrPaths.locateSolrHome().toAbsolutePath().normalize();
log.warn("SolrResourceLoader created with null instanceDir. This will not be supported in Solr 9.0");
@@ -274,12 +276,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
* @throws IOException on error
*/
public static List<URL> getURLs(Path libDir) throws IOException {
- return getURLs(libDir, new DirectoryStream.Filter<Path>() {
- @Override
- public boolean accept(Path entry) throws IOException {
- return true;
- }
- });
+ return getURLs(libDir, entry -> true);
}
/**
@@ -291,12 +288,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
*/
public static List<URL> getFilteredURLs(Path libDir, String regex) throws IOException {
final PathMatcher matcher = libDir.getFileSystem().getPathMatcher("regex:" + regex);
- return getURLs(libDir, new DirectoryStream.Filter<Path>() {
- @Override
- public boolean accept(Path entry) throws IOException {
- return matcher.matches(entry.getFileName());
- }
- });
+ return getURLs(libDir, entry -> matcher.matches(entry.getFileName()));
}
/** Ensures a directory name always ends with a '/'. */
@@ -357,16 +349,6 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
return openResource(name);
}
- 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:
* $configDir/$resource (if resource is not absolute)
@@ -377,15 +359,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).
@@ -408,13 +396,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.toAbsolutePath().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.toAbsolutePath().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)
@@ -423,7 +418,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
// ignore
}
- return resource;
+ return allowUnsafeResourceloading ? resource : null;
}
/**
@@ -509,7 +504,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
}
}
- Class<? extends T> clazz = null;
+ Class<? extends T> clazz;
clazz = getPackageClass(cname, expectedType);
if(clazz != null) return clazz;
try {
@@ -615,10 +610,10 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
"Can not find class: "+cName + " in " + classLoader);
}
- T obj = null;
+ T obj;
try {
- Constructor<? extends T> constructor = null;
+ Constructor<? extends T> constructor;
try {
constructor = clazz.getConstructor(params);
obj = constructor.newInstance(args);
@@ -973,7 +968,7 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, msg);
}
}
- try (OutputStream out = new FileOutputStream(confFile);) {
+ try (OutputStream out = new FileOutputStream(confFile)) {
out.write(content);
}
log.info("Written confile {}", resourceName);
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 884751b..4708c9b 100644
--- a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
+++ b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
@@ -314,7 +314,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 69e8ad0..770ba08 100644
--- a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
+++ b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java
@@ -41,11 +41,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.*;
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();
@@ -61,13 +68,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"})
@@ -131,10 +163,10 @@ public class ResourceLoaderTest extends SolrTestCaseJ4 {
List<String> lines = loader.getLines(fileWithBom);
assertEquals(1, lines.size());
assertEquals("BOMsAreEvil", lines.get(0));
-
+
loader.close();
}
-
+
public void testWrongEncoding() throws Exception {
String wrongEncoding = "stopwordsWrongEncoding.txt";
try(SolrResourceLoader loader = new SolrResourceLoader(TEST_PATH().resolve("collection1"))) {
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 30e3f70..3603e4a 100644
--- a/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java
@@ -42,7 +42,6 @@ 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");
initMap = new HashMap<>();
config = new SolrConfig(TEST_PATH().resolve("collection1"), testConfHome + "solrconfig.xml");
@@ -50,7 +49,6 @@ public class PrimitiveFieldTypeTest extends SolrTestCaseJ4 {
@Override
public void tearDown() throws Exception {
- System.clearProperty("solr.allow.unsafe.resourceloading");
super.tearDown();
}