[GitHub] [commons-io] pjfanning commented on pull request #371: [SECURITY] Fix Partial Path Traversal Vulnerability

[GitBox <gi...@apache.org>]

pjfanning commented on PR #371:
URL: https://github.com/apache/commons-io/pull/371#issuecomment-1228927498

   This change affects code that tries to exclude some files when copying a directory (to avoid https://issues.apache.org/jira/browse/IO-141). I can see how the change can help to avoid stopping some files being added to the exclusion list when they shouldn't. I tried to take the existing FileUtilsTest for IO-141 (testCopyDirectoryToChild) and to add one based on it but where the source dir is not really a parent dir of the dest dir but the existing check mistakenly thinks it is. I have such a test (below) but it works with and without this PR change.
   
   I don't think there is any security issue here.
   
   ```
       /**
        * Test for https://github.com/apache/commons-io/pull/371. The dir name 'par' is a substring of
        * the dir name 'parent' which is the parent of the 'parent/child' dir.
        */
       @Test
       public void testCopyDirectoryWithPotentialFalsePartialMatch() throws Exception {
           final File grandParentDir = new File(tempDirFile, "grandparent");
           final File parentDir = new File(grandParentDir, "parent");
           final File parDir = new File(grandParentDir, "par");
           final File childDir = new File(parentDir, "child");
           createFilesForTestCopyDirectory(grandParentDir, parDir, childDir);
   
           final List<File> initFiles = LIST_WALKER.list(grandParentDir);
           final List<File> parFiles = LIST_WALKER.list(parDir);
           final long expectedCount = initFiles.size() + parFiles.size();
           final long expectedSize = FileUtils.sizeOfDirectory(grandParentDir) + FileUtils.sizeOfDirectory(parDir);
           FileUtils.copyDirectory(parDir, childDir);
           final List<File> latestFiles = LIST_WALKER.list(grandParentDir);
           assertEquals(expectedCount, latestFiles.size());
           assertEquals(expectedSize, FileUtils.sizeOfDirectory(grandParentDir));
           assertTrue(expectedCount > 0, "Count > 0");
           assertTrue(expectedSize > 0, "Size > 0");
           Set<String> initFilePaths = getFilePathSet(initFiles);
           Set<String> newFilePaths = getFilePathSet(latestFiles);
           newFilePaths.removeAll(initFilePaths);
           assertEquals(parFiles.size(), newFilePaths.size());
       }
   
       private Set<String> getFilePathSet(List<File> files) {
           return files.stream().map(f -> {
               try {
                   return f.getCanonicalPath();
               } catch (IOException e) {
                   throw new RuntimeException(e);
               }
           }).collect(Collectors.toSet());
       }
   ```


-- 
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-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org