Fix path traversal bug
VM names are used as directory names, we shouldn't allow path
separators in them.
This isn't actually a security issue (we're running in the app's
context, we'll never create any files or directories that the app
couldn't just create itself), but it still seems like a bad thing.
Test: atest MicrodroidTests
Change-Id: I7eec7963b15050239c14d7ef989c6bb623d06b2c
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index d774cec..ffcdc51 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -418,7 +418,10 @@
}
@NonNull
- private static File getVmDir(Context context, String name) {
+ private static File getVmDir(@NonNull Context context, @NonNull String name) {
+ if (name.contains(File.separator) || name.equals(".") || name.equals("..")) {
+ throw new IllegalArgumentException("Invalid VM name: " + name);
+ }
File vmRoot = new File(context.getDataDir(), VM_DIR);
return new File(vmRoot, name);
}
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
index c179498..ea0a305 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
@@ -43,6 +43,7 @@
*
* <p>Each virtual machine instance is named; the configuration and related state of each is
* persisted in the app's private data directory and an instance can be retrieved given the name.
+ * The name must be a valid directory name and must not contain '/'.
*
* <p>The app can then start, stop and otherwise interact with the VM.
*
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index 899e47e..fc579d8 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -318,8 +318,6 @@
"9.17/C-1-1",
})
public void invalidApkPathIsRejected() {
- assumeSupportedKernel();
-
VirtualMachineConfig.Builder builder =
newVmConfigBuilder()
.setPayloadBinaryPath("MicrodroidTestNativeLib.so")
@@ -329,6 +327,14 @@
}
@Test
+ @CddTest(requirements = {"9.17/C-1-1"})
+ public void invalidVmNameIsRejected() {
+ VirtualMachineManager vmm = getVirtualMachineManager();
+ assertThrows(IllegalArgumentException.class, () -> vmm.get("../foo"));
+ assertThrows(IllegalArgumentException.class, () -> vmm.get(".."));
+ }
+
+ @Test
@CddTest(requirements = {
"9.17/C-1-1",
"9.17/C-2-1"