joel-bernstein commented on a change in pull request #1600:
URL: https://github.com/apache/lucene-solr/pull/1600#discussion_r571634497
##########
File path: solr/core/src/java/org/apache/solr/handler/CatStream.java
##########
@@ -158,28 +157,20 @@ public Explanation toExplanation(StreamFactory factory)
throws IOException {
}
private List<CrawlFile> validateAndSetFilepathsInSandbox() {
- final String[] relativePathRoots = commaDelimitedFilepaths.split(",");
-
final List<CrawlFile> crawlSeeds = new ArrayList<>();
- for (String crawlRoot : relativePathRoots) {
- final File crawlRootFile = new File(crawlRoot);
- if (crawlRootFile.isAbsolute()) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "file/directory to stream must be provided as an absolute path: "
+ crawlRoot);
- }
- if ( crawlRoot.contains("..")) {
+ for (String crawlRootStr : commaDelimitedFilepaths.split(",")) {
+ Path crawlRootPath = chroot.resolve(crawlRootStr).normalize();
+ if (! crawlRootPath.startsWith(chroot)) {
Review comment:
I hope you're right about this. In my testing it does prevent the use of
"..", but there is nothing explicit that I've read that says the change made
here will prevent the use of "..". This is not a small issue because of the
security ramifications.
This PR also exposes the chroot in the error message which needs to be
changed.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
