🔒 Fix Arbitrary File Overwrite via Intent

app/build.gradle

@@ -2,7 +2,11 @@ apply plugin: 'com.android.application'
 
 android {
     compileSdk 35
-    compileOptions.encoding = 'UTF-8'
+    compileOptions {
+        encoding = 'UTF-8'
+        sourceCompatibility JavaVersion.VERSION_17
+        targetCompatibility JavaVersion.VERSION_17
+    }
 
     defaultConfig {
         applicationId "io.github.ysdede.opencameraomt"
@@ -105,9 +109,3 @@ dependencies {
     androidTestImplementation "androidx.test:rules:1.7.0"
     androidTestImplementation "androidx.test.espresso:espresso-core:3.7.0"
 }
-
-java {
-    toolchain {
-        languageVersion = JavaLanguageVersion.of(17)
-    }
-}

app/src/main/java/net/sourceforge/opencamera/MyApplicationInterface.java

@@ -337,7 +337,13 @@ public VideoMethod createOutputVideoMethod() {
                 if (intent_uri != null) {
                     if (MyDebug.LOG)
                         Log.d(TAG, "save to: " + intent_uri);
-                    return VideoMethod.URI;
+                    if (!storageUtils.isUriSafe(intent_uri)) {
+                        Log.e(TAG, "not saving to intent_uri as it is not safe: " + intent_uri);
+                        intent_uri = null;
+                    }
+                    else {
+                        return VideoMethod.URI;
+                    }
                 }
             }
             // if no EXTRA_OUTPUT, we should save to standard location, and will pass back

app/src/main/java/net/sourceforge/opencamera/MyDebug.java

@@ -8,7 +8,7 @@ public class MyDebug {
     /** Global constant to control logging, should always be set to false in
      *  released versions.
      */
-    public static final boolean LOG = true;
+    public static final boolean LOG = false;
 
     /** Wrapper to print exceptions, should use instead of e.printStackTrace().
      */

app/src/main/java/net/sourceforge/opencamera/StorageUtils.java

@@ -1686,4 +1686,71 @@ public long freeMemory() { // return free memory in MB
         }
         return -1;
     }
+
+    /** Checks if the URI is safe to write to.
+     *  It checks if the URI points to a file that is within the application's private data directory.
+     */
+    boolean isUriSafe(Uri uri) {
+        if( MyDebug.LOG )
+            Log.d(TAG, "isUriSafe: " + uri);
+        if( uri == null )
+            return false;
+        String scheme = uri.getScheme();
+        String path = null;
+        if( scheme != null && scheme.equalsIgnoreCase("file") ) {
+            path = uri.getPath();
+        }
+        else if( scheme != null && scheme.equalsIgnoreCase("content") ) {
+            // try to resolve path
+            path = getDataColumn(uri, null, null);
+        }
+
+        if( path != null ) {
+            String dataDir = context.getApplicationInfo().dataDir;
+            return isPathSafe(path, dataDir);
+        }
+
+        return true;
+    }
+
+    /** Visible for testing.
+     *  Checks if the path is safe (i.e. not inside the dataDir).
+     */
+    public boolean isPathSafe(String path, String dataDir) {
+        if( MyDebug.LOG ) {
+            Log.d(TAG, "isPathSafe: " + path);
+            Log.d(TAG, "dataDir: " + dataDir);
+        }
+        if( path == null )
+            return true; // Can't check, assume safe (or handled elsewhere)
+        if( dataDir == null )
+            return true; // Can't check
+
+        try {
+            File file = new File(path);
+            File dataDirFile = new File(dataDir);
+            // We use canonical paths to resolve .. etc
+            String canonicalPath = file.getCanonicalPath();
+            String canonicalDataDir = dataDirFile.getCanonicalPath();
+
+            // Check if canonicalPath starts with canonicalDataDir
+            // We append File.separator to ensure we don't match partial directory names (e.g. /data/data/com.foo vs /data/data/com.foobar)
+            if( !canonicalDataDir.endsWith(File.separator) )
+                canonicalDataDir += File.separator;
+
+            if( canonicalPath.startsWith(canonicalDataDir) ) {
+                if( MyDebug.LOG )
+                    Log.e(TAG, "path is inside dataDir: " + canonicalPath);
+                return false;
+            }
+        }
+        catch(IOException e) {
+            MyDebug.logStackTrace(TAG, "failed to get canonical path", e);
+            // If we can't get canonical path, checking is hard.
+            // If path contains "..", it might be an attack.
+            if( path.contains("..") )
+                return false;
+        }
+        return true;
+    }
 }

app/src/test/java/net/sourceforge/opencamera/SecurityTest.java

@@ -0,0 +1,53 @@
+package net.sourceforge.opencamera;
+
+import org.junit.Test;
+import static org.junit.Assert.*;
+import java.io.File;
+import java.io.IOException;
+
+public class SecurityTest {
+
+    @Test
+    public void testPathSafety() throws IOException {
+        // We can pass nulls to constructor since isPathSafe doesn't use instance members.
+        StorageUtils storageUtils = new StorageUtils(null, null);
+
+        String tempDir = System.getProperty("java.io.tmpdir");
+        if (tempDir == null) tempDir = "/tmp";
+
+        // Use a subdirectory as the mock data dir
+        File mockDataDir = new File(tempDir, "mockDataDir");
+        String mockDataDirPath = mockDataDir.getCanonicalPath();
+
+        // 1. Path inside data dir -> Unsafe
+        File unsafeFile = new File(mockDataDir, "config.xml");
+        assertFalse("File inside data dir should be unsafe",
+            storageUtils.isPathSafe(unsafeFile.getAbsolutePath(), mockDataDirPath));
+
+        // 2. Path outside data dir -> Safe
+        File safeFile = new File(tempDir, "photo.jpg");
+        // Ensure safeFile is actually outside mockDataDir (it should be since it's in parent)
+        if (!safeFile.getCanonicalPath().startsWith(mockDataDirPath)) {
+            assertTrue("File outside data dir should be safe",
+                storageUtils.isPathSafe(safeFile.getAbsolutePath(), mockDataDirPath));
+        }
+
+        // 3. Traversal out of data dir -> Safe
+        // /tmp/mockDataDir/../photo.jpg -> /tmp/photo.jpg
+        File traversalSafe = new File(mockDataDir, "../photo.jpg");
+        assertTrue("Traversal out of data dir should be safe",
+            storageUtils.isPathSafe(traversalSafe.getAbsolutePath(), mockDataDirPath));
+
+        // 4. Sibling directory with similar prefix -> Safe
+        // /tmp/mockDataDirSuffix
+        File siblingDir = new File(tempDir, "mockDataDirSuffix");
+        String siblingDirPath = siblingDir.getAbsolutePath();
+        assertTrue("Sibling dir with similar prefix should be safe",
+            storageUtils.isPathSafe(siblingDirPath, mockDataDirPath));
+
+        // 5. Nested path inside data dir -> Unsafe
+        File nestedUnsafe = new File(mockDataDir, "subdir/secret.db");
+        assertFalse("Nested file inside data dir should be unsafe",
+            storageUtils.isPathSafe(nestedUnsafe.getAbsolutePath(), mockDataDirPath));
+    }
+}