janhoy commented on a change in pull request #1572:
URL: https://github.com/apache/lucene-solr/pull/1572#discussion_r441858025
##########
File path: solr/core/src/java/org/apache/solr/core/SolrPaths.java
##########
@@ -128,4 +130,35 @@ private static void logOnceInfo(String key, String msg) {
log.info(msg);
}
}
+
+ /**
+ * Checks that the given path is relative to SOLR_HOME, SOLR_DATA_HOME,
coreRootDirectory or one of the paths
+ * specified in solr.xml's allowPaths element. The following paths will fail
validation
+ * <ul>
+ * <li>Relative paths starting with <code>..</code></li>
+ * <li>Windows UNC paths (<code>\\host\share\path</code>)</li>
+ * <li>Absolute paths which are not below the list of allowed paths</li>
+ * </ul>
+ * @param pathToAssert path to check
+ * @param allowPaths list of paths that should be allowed prefixes
+ * @throws SolrException if path is outside allowed paths
+ */
+ public static void assertPathAllowed(Path pathToAssert, Set<Path>
allowPaths) throws SolrException {
+ if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Path " + pathToAssert + " disallowed. UNC paths not supported.
Please use drive letter instead.");
+ }
+ // Conversion Path -> String -> Path is to be able to compare against
org.apache.lucene.mockfile.FilterPath instances
+ final Path path = Path.of(pathToAssert.toString()).normalize();
+ if (path.startsWith("..")) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Path " + pathToAssert + " disallowed due to path traversal..");
+ }
+ if (!path.isAbsolute()) return; // All relative paths are accepted
+ if (allowPaths.contains(Paths.get("_ALL_"))) return; // Catch-all path
"*"/"_ALL_" will allow all other paths
Review comment:
This is the workaround I did after realizing that Windows `Path` class
is not happy with `*` as a path. When parsing the value from solr.xml/sysprop,
we detect `*` and store it as a Path `_ALL_`. Then in the assert method we
check for that special path and skip further testing.
Exception is UNC paths and `..` paths which are still rejected (should they?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
