Only accept binary name not path
I think we've discussed this a couple of times, although I can't now
find a link.
There's really no reason to specify a path, it's complicated to
describe, and it might open up weird path traversal attacks,so
disallow it.
Rename setPayloadBinaryPath to setPayloadBinaryName to reflect this
(and rename lots of other things to match). Add a check that it isn't
a path, and a test for that (and fix some other tests that were
breaking the new rule).
Also expand on the Javadoc around ABI & 32/64-bit.
Also add a check inside VS (because checks in the payload code can be
bypassed), and a host test for that.
Note that a VM created with a config file can still specify a path
inside the config file; CompOS relies on that to run code from its
APEX.
Bug: 261037705
Test: atest MicrodroidTests MicrodroidHostTests
Change-Id: Ie59b9c81d13a7a3e4ec62cf874d43bfaf6163431
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index 30e437b..71bdf13 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -61,7 +61,7 @@
method @IntRange(from=0) public long getEncryptedStorageKib();
method @IntRange(from=0) public int getMemoryMib();
method @IntRange(from=1) public int getNumCpus();
- method @Nullable public String getPayloadBinaryPath();
+ method @Nullable public String getPayloadBinaryName();
method public boolean isCompatibleWith(@NonNull android.system.virtualmachine.VirtualMachineConfig);
method public boolean isEncryptedStorageEnabled();
method public boolean isProtectedVm();
@@ -77,7 +77,7 @@
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setEncryptedStorageKib(@IntRange(from=1) long);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setMemoryMib(@IntRange(from=1) int);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setNumCpus(@IntRange(from=1) int);
- method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setPayloadBinaryPath(@NonNull String);
+ method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setPayloadBinaryName(@NonNull String);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setProtectedVm(boolean);
}
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 34851e4..e319aa3 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -1226,17 +1226,15 @@
public String toString() {
VirtualMachineConfig config = getConfig();
String payloadConfigPath = config.getPayloadConfigPath();
- String payloadBinaryPath = config.getPayloadBinaryPath();
+ String payloadBinaryName = config.getPayloadBinaryName();
StringBuilder result = new StringBuilder();
result.append("VirtualMachine(")
.append("name:")
.append(getName())
.append(", ");
- if (payloadBinaryPath != null) {
- result.append("payload:")
- .append(payloadBinaryPath)
- .append(", ");
+ if (payloadBinaryName != null) {
+ result.append("payload:").append(payloadBinaryName).append(", ");
}
if (payloadConfigPath != null) {
result.append("config:")
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index 7c16798..0e9e86b 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -62,7 +62,7 @@
private static final String KEY_VERSION = "version";
private static final String KEY_APKPATH = "apkPath";
private static final String KEY_PAYLOADCONFIGPATH = "payloadConfigPath";
- private static final String KEY_PAYLOADBINARYPATH = "payloadBinaryPath";
+ private static final String KEY_PAYLOADBINARYNAME = "payloadBinaryPath";
private static final String KEY_DEBUGLEVEL = "debugLevel";
private static final String KEY_PROTECTED_VM = "protectedVm";
private static final String KEY_MEMORY_MIB = "memoryMib";
@@ -118,10 +118,8 @@
*/
@Nullable private final String mPayloadConfigPath;
- /**
- * Path within the APK to the payload binary file that will be executed within the VM.
- */
- @Nullable private final String mPayloadBinaryPath;
+ /** Name of the payload binary file within the APK that will be executed within the VM. */
+ @Nullable private final String mPayloadBinaryName;
/** The size of storage in KiB. 0 indicates that encryptedStorage is not required */
private final long mEncryptedStorageKib;
@@ -129,7 +127,7 @@
private VirtualMachineConfig(
@NonNull String apkPath,
@Nullable String payloadConfigPath,
- @Nullable String payloadBinaryPath,
+ @Nullable String payloadBinaryName,
@DebugLevel int debugLevel,
boolean protectedVm,
int memoryMib,
@@ -138,7 +136,7 @@
// This is only called from Builder.build(); the builder handles parameter validation.
mApkPath = apkPath;
mPayloadConfigPath = payloadConfigPath;
- mPayloadBinaryPath = payloadBinaryPath;
+ mPayloadBinaryName = payloadBinaryName;
mDebugLevel = debugLevel;
mProtectedVm = protectedVm;
mMemoryMib = memoryMib;
@@ -192,7 +190,7 @@
String payloadConfigPath = b.getString(KEY_PAYLOADCONFIGPATH);
if (payloadConfigPath == null) {
- builder.setPayloadBinaryPath(b.getString(KEY_PAYLOADBINARYPATH));
+ builder.setPayloadBinaryName(b.getString(KEY_PAYLOADBINARYNAME));
} else {
builder.setPayloadConfigPath(payloadConfigPath);
}
@@ -231,7 +229,7 @@
b.putInt(KEY_VERSION, VERSION);
b.putString(KEY_APKPATH, mApkPath);
b.putString(KEY_PAYLOADCONFIGPATH, mPayloadConfigPath);
- b.putString(KEY_PAYLOADBINARYPATH, mPayloadBinaryPath);
+ b.putString(KEY_PAYLOADBINARYNAME, mPayloadBinaryName);
b.putInt(KEY_DEBUGLEVEL, mDebugLevel);
b.putBoolean(KEY_PROTECTED_VM, mProtectedVm);
b.putInt(KEY_NUM_CPUS, mNumCpus);
@@ -269,15 +267,15 @@
}
/**
- * Returns the path within the {@code lib/<ABI>} directory of the APK to the payload binary file
+ * Returns the name of the payload binary file, in the {@code lib/<ABI>} directory of the APK,
* that will be executed within the VM.
*
* @hide
*/
@SystemApi
@Nullable
- public String getPayloadBinaryPath() {
- return mPayloadBinaryPath;
+ public String getPayloadBinaryName() {
+ return mPayloadBinaryName;
}
/**
@@ -362,7 +360,7 @@
&& this.mProtectedVm == other.mProtectedVm
&& this.mEncryptedStorageKib == other.mEncryptedStorageKib
&& Objects.equals(this.mPayloadConfigPath, other.mPayloadConfigPath)
- && Objects.equals(this.mPayloadBinaryPath, other.mPayloadBinaryPath)
+ && Objects.equals(this.mPayloadBinaryName, other.mPayloadBinaryName)
&& this.mApkPath.equals(other.mApkPath);
}
@@ -376,9 +374,9 @@
VirtualMachineAppConfig toVsConfig() throws FileNotFoundException {
VirtualMachineAppConfig vsConfig = new VirtualMachineAppConfig();
vsConfig.apk = ParcelFileDescriptor.open(new File(mApkPath), MODE_READ_ONLY);
- if (mPayloadBinaryPath != null) {
+ if (mPayloadBinaryName != null) {
VirtualMachinePayloadConfig payloadConfig = new VirtualMachinePayloadConfig();
- payloadConfig.payloadPath = mPayloadBinaryPath;
+ payloadConfig.payloadBinaryName = mPayloadBinaryName;
vsConfig.payload =
VirtualMachineAppConfig.Payload.payloadConfig(payloadConfig);
} else {
@@ -411,7 +409,7 @@
@Nullable private final Context mContext;
@Nullable private String mApkPath;
@Nullable private String mPayloadConfigPath;
- @Nullable private String mPayloadBinaryPath;
+ @Nullable private String mPayloadBinaryName;
@DebugLevel private int mDebugLevel = DEBUG_LEVEL_NONE;
private boolean mProtectedVm;
private boolean mProtectedVmSet;
@@ -455,14 +453,14 @@
apkPath = mApkPath;
}
- if (mPayloadBinaryPath == null) {
+ if (mPayloadBinaryName == null) {
if (mPayloadConfigPath == null) {
- throw new IllegalStateException("setPayloadBinaryPath must be called");
+ throw new IllegalStateException("setPayloadBinaryName must be called");
}
} else {
if (mPayloadConfigPath != null) {
throw new IllegalStateException(
- "setPayloadBinaryPath and setPayloadConfigPath may not both be called");
+ "setPayloadBinaryName and setPayloadConfigPath may not both be called");
}
}
@@ -473,7 +471,7 @@
return new VirtualMachineConfig(
apkPath,
mPayloadConfigPath,
- mPayloadBinaryPath,
+ mPayloadBinaryName,
mDebugLevel,
mProtectedVm,
mMemoryMib,
@@ -515,16 +513,23 @@
}
/**
- * Sets the path within the {@code lib/<ABI>} directory of the APK to the payload binary
- * file that will be executed within the VM.
+ * Sets the name of the payload binary file that will be executed within the VM, e.g.
+ * "payload.so". The file must reside in the {@code lib/<ABI>} directory of the APK.
+ *
+ * <p>Note that VMs only support 64-bit code, even if the owning app is running as a 32-bit
+ * process.
*
* @hide
*/
@SystemApi
@NonNull
- public Builder setPayloadBinaryPath(@NonNull String payloadBinaryPath) {
- mPayloadBinaryPath =
- requireNonNull(payloadBinaryPath, "payloadBinaryPath must not be null");
+ public Builder setPayloadBinaryName(@NonNull String payloadBinaryName) {
+ if (payloadBinaryName.contains(File.separator)) {
+ throw new IllegalArgumentException(
+ "Invalid binary file name: " + payloadBinaryName);
+ }
+ mPayloadBinaryName =
+ requireNonNull(payloadBinaryName, "payloadBinaryName must not be null");
return this;
}