commit 8f12f2b7aa3bec1132ade6146a548991830bd111
author Alan Stokes <alanstokes@google.com> Mon Jan 09 09:19:20 2023 +0000
committer Alan Stokes <alanstokes@google.com> Mon Jan 09 16:57:42 2023 +0000
tree a0cf5cf42f5c6eb58f3b448e2e52d02744dbe05a
parent 5d786d39273a9971a0b46ec28e438f7a0a2b537b

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

vm/src/run.rs

diff --git a/vm/src/run.rs b/vm/src/run.rs
index 6096913..b99328a 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -48,7 +48,7 @@
     storage: Option<&Path>,
     storage_size: Option<u64>,
     config_path: Option<String>,
-    payload_path: Option<String>,
+    payload_binary_name: Option<String>,
     daemonize: bool,
     console_path: Option<&Path>,
     log_path: Option<&Path>,
@@ -117,14 +117,16 @@
     let extra_idsig_fds = extra_idsig_files?.into_iter().map(ParcelFileDescriptor::new).collect();
 
     let payload = if let Some(config_path) = config_path {
-        if payload_path.is_some() {
-            bail!("Only one of --config-path or --payload-path can be defined")
+        if payload_binary_name.is_some() {
+            bail!("Only one of --config-path or --payload-binary-name can be defined")
         }
         Payload::ConfigPath(config_path)
-    } else if let Some(payload_path) = payload_path {
-        Payload::PayloadConfig(VirtualMachinePayloadConfig { payloadPath: payload_path })
+    } else if let Some(payload_binary_name) = payload_binary_name {
+        Payload::PayloadConfig(VirtualMachinePayloadConfig {
+            payloadBinaryName: payload_binary_name,
+        })
     } else {
-        bail!("Either --config-path or --payload-path must be defined")
+        bail!("Either --config-path or --payload-binary-name must be defined")
     };
 
     let payload_config_str = format!("{:?}!{:?}", apk, payload);
@@ -197,7 +199,7 @@
     let instance_img = work_dir.join("instance.img");
     println!("instance.img path: {}", instance_img.display());
 
-    let payload_path = "MicrodroidEmptyPayloadJniLib.so";
+    let payload_binary_name = "MicrodroidEmptyPayloadJniLib.so";
     let extra_sig = [];
     command_run_app(
         name,
@@ -208,7 +210,7 @@
         storage,
         storage_size,
         /* config_path= */ None,
-        Some(payload_path.to_owned()),
+        Some(payload_binary_name.to_owned()),
         daemonize,
         console_path,
         log_path,