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/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 733d9c5..0859a76 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -841,7 +841,7 @@
load_vm_payload_config_from_file(&apk_file, config_path.as_str())
.with_context(|| format!("Couldn't read config from {}", config_path))?
}
- Payload::PayloadConfig(payload_config) => create_vm_payload_config(payload_config),
+ Payload::PayloadConfig(payload_config) => create_vm_payload_config(payload_config)?,
};
// For now, the only supported OS is Microdroid
@@ -887,13 +887,20 @@
Ok(serde_json::from_reader(config_file)?)
}
-fn create_vm_payload_config(payload_config: &VirtualMachinePayloadConfig) -> VmPayloadConfig {
+fn create_vm_payload_config(
+ payload_config: &VirtualMachinePayloadConfig,
+) -> Result<VmPayloadConfig> {
// There isn't an actual config file. Construct a synthetic VmPayloadConfig from the explicit
// parameters we've been given. Microdroid will do something equivalent inside the VM using the
// payload config that we send it via the metadata file.
- let task =
- Task { type_: TaskType::MicrodroidLauncher, command: payload_config.payloadPath.clone() };
- VmPayloadConfig {
+
+ let payload_binary_name = &payload_config.payloadBinaryName;
+ if payload_binary_name.contains('/') {
+ bail!("Payload binary name must not specify a path: {payload_binary_name}");
+ }
+
+ let task = Task { type_: TaskType::MicrodroidLauncher, command: payload_binary_name.clone() };
+ Ok(VmPayloadConfig {
os: OsConfig { name: MICRODROID_OS_NAME.to_owned() },
task: Some(task),
apexes: vec![],
@@ -901,7 +908,7 @@
prefer_staged: false,
export_tombstones: false,
enable_authfs: false,
- }
+ })
}
/// Generates a unique filename to use for a composite disk image.
diff --git a/virtualizationservice/src/payload.rs b/virtualizationservice/src/payload.rs
index eb3e9eb..02e8f8e 100644
--- a/virtualizationservice/src/payload.rs
+++ b/virtualizationservice/src/payload.rs
@@ -194,7 +194,7 @@
) -> Result<ParcelFileDescriptor> {
let payload_metadata = match &app_config.payload {
Payload::PayloadConfig(payload_config) => PayloadMetadata::config(PayloadConfig {
- payload_binary_path: payload_config.payloadPath.clone(),
+ payload_binary_name: payload_config.payloadBinaryName.clone(),
..Default::default()
}),
Payload::ConfigPath(config_path) => {