janhoy commented on a change in pull request #593:
URL: https://github.com/apache/solr/pull/593#discussion_r798948776
##########
File path: solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
##########
@@ -94,8 +94,18 @@ private static Path _getRealPath(String path, Path solrHome)
{
if (!path.isEmpty() && path.charAt(0) != File.separatorChar) {
path = File.separator + path;
}
- // Use concat because path might start with a slash and be incorrectly
interpreted as absolute
- return solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY + path);
+ if (path.startsWith("\\\\")) { // Windows absolute UNC
+ throw new SolrException(BAD_REQUEST, "Illegal path " + path);
+ }
+ while (path.startsWith("/")) { // Trim all leading slashes
+ path = path.substring(1);
+ }
+ var finalPath = getPackageStoreDirPath(solrHome).resolve(path);
+ // Guard against path traversal by asserting final path is sub path of
filestore
+ if
(finalPath.normalize().startsWith(getPackageStoreDirPath(solrHome).normalize()))
{
Review comment:
If a path is `/foo/bar/../../baz` then the normalized is `/baz`.
Probably not necessary, but I thought that if the user-input path had path
traversal, then e.g. `PACKAGESTORE_PATH.resolve(../../../../foo)` would still
return true for `PACKAGESTORE_PATH.startsWith()`, but if you normalize it will
fail since it does not share the same root.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
