Butterfly has path/URL confusion in resource handling leading to multiple weaknesses

@@ -14,6 +14,7 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
+import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
@@ -71,6 +72,7 @@ public class ButterflyModuleImpl implements ButterflyModule {
     protected Timer _timer;
     protected ServletConfig _config;
     protected File _path;
+    protected Path _normalizedPath;
     protected MountPoint _mountPoint;
     protected ButterflyMounter _mounter;
     protected String _name;
@@ -118,6 +120,7 @@ public void setClassLoader(ClassLoader classLoader) {
     public void setPath(File path) {
         _logger.trace("{} -(path)-> {}", this, path);
         this._path = path;
+        this._normalizedPath = path.toPath().toAbsolutePath().normalize();
     }
 
     public void setName(String name) {
@@ -259,6 +262,8 @@ public ButterflyModule getModule(String name) {
 
     protected Pattern super_pattern = Pattern.compile("^@@(.*)@@$");
     
+    // TODO 2025-10: migrate away from URL as a return type to File/Path as we don't want this to fetch anything remote
+    @Override
     public URL getResource(String resource) {
         _logger.trace("> getResource({}->{},{})", new Object[] { _name, _extended, resource });
         URL u = null;
@@ -283,14 +288,11 @@ public URL getResource(String resource) {
         
         if (u == null) {
             try {
-                if (resource.startsWith("file:/")) {
-                    u = new URL(resource);
-                } else {
-                    if (resource.charAt(0) == '/') resource = resource.substring(1);
-                    File f = new File(_path, resource);
-                    if (f.exists()) {
-                        u = f.toURI().toURL();
-                    }
+                if (resource.charAt(0) == '/') resource = resource.substring(1);
+                File f = new File(_path, resource);
+                // check that the file does not escape the expected directory
+                if (f.toPath().toAbsolutePath().normalize().startsWith(_normalizedPath) && f.exists()) {
+                    u = f.toURI().toURL();
                 }
             } catch (MalformedURLException e) {
                 _logger.error("Error", e);

@@ -0,0 +1,53 @@
+package edu.mit.simile.butterfly.tests;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.MalformedURLException;
+import java.nio.file.Files;
+
+import edu.mit.simile.butterfly.ButterflyModuleImpl;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class ButterflyModuleImplTests {
+    
+    ButterflyModuleImpl SUT;
+    File tempDir;
+    File firstFolder;
+    File secondFolder;
+    File textFile;
+    File testFile;
+    
+    @BeforeMethod
+    public void setUp() throws IOException {
+        SUT = new ButterflyModuleImpl();
+        tempDir = Files.createTempDirectory("ButterflyModuleImplTests").toFile();
+        tempDir.deleteOnExit();
+        firstFolder = new File(tempDir, "first_folder");
+        firstFolder.mkdir();
+        secondFolder = new File(tempDir, "other_folder");
+        secondFolder.mkdir();
+        textFile = new File(secondFolder, "file.txt");
+        textFile.createNewFile();
+        testFile = new File(firstFolder, "test.txt");
+        testFile.createNewFile();
+        SUT.setPath(firstFolder);
+    }
+    
+    @Test
+    public void testGetResource() throws MalformedURLException {
+        // file exists and is in the expected directory
+        Assert.assertEquals(SUT.getResource("test.txt"), testFile.toURI().toURL());
+        // file does not exist
+        Assert.assertNull(SUT.getResource("does_not_exist.xls"));
+        
+        // file exists but escapes the expected directory (it would be a security issue to accept it)
+        Assert.assertEquals(SUT.getResource("../other_folder/file.txt"), null);
+        // we don't support passing full URIs (it would be a security issue to accept reading any resource)
+        String fullURI = testFile.toURI().toString();
+        Assert.assertTrue(fullURI.startsWith("file:/"));
+        Assert.assertEquals(SUT.getResource(fullURI), null);
+    }
+
+}