Disable environment variables and arguments by default in guests (#2415)

This PR disables syscalls `sys_getenv`, `sys_argc` and `sys_argv` by
default, with the option to enable them via a feature flag on the
`risc0-zkvm-platform` crate. In host programs, environment variables and
arguments are generally considered trusted. In contrast, the guest does
not trust the host, which leads to risk that code designed for running
on the host may result in insecure behavior when run in the guest.
Disabling environment variables and args by default mitigates this risk.

.vim/coc-settings.json

@@ -3,6 +3,7 @@
     "Cargo.toml",
     "benchmarks/Cargo.toml",
     "examples/Cargo.toml",
+    "risc0/cargo-risczero/tests/test_crate",
     "risc0/zkvm/methods/guest/Cargo.toml",
     "risc0/zkvm/methods/std/Cargo.toml",
     "tools/crates-validator/Cargo.toml"

.vscode/settings.json

@@ -62,6 +62,7 @@
     "Cargo.toml",
     "benchmarks/Cargo.toml",
     "examples/Cargo.toml",
+    "risc0/cargo-risczero/tests/test_crate",
     "risc0/zkvm/methods/guest/Cargo.toml",
     "risc0/zkvm/methods/std/Cargo.toml",
     "tools/crates-validator/Cargo.toml"

CHANGELOG.md

@@ -2,7 +2,17 @@
 
 ## Next (upcoming release)
 
-TBD
+### Fixes
+
+### 🚨 Breaking Changes
+
+* Environment syscalls `sys_getenv`, `sys_argc` and `sys_argv` are disabled by
+  default, with the option to enable them via a feature flag on the
+  `risc0-zkvm-platform` crate. In host programs, environment variables and
+  arguments are generally considered trusted. In contrast, the guest does not
+  trust the host, which leads to risk that code designed for running on the
+  host may result in insecure behavior when run in the guest. Disabling
+  environment variables and args by default mitigates this risk.
 
 ## [v1.1.0 (2024-09-09)](https://github.com/risc0/risc0/releases/tag/v1.1.0)
 

examples/xgboost/methods/guest/Cargo.lock

@@ -1137,6 +1137,7 @@ version = "0.1.0"
 dependencies = [
  "forust-ml",
  "risc0-zkvm",
+ "risc0-zkvm-platform",
  "rmp-serde",
 ]
 

examples/xgboost/methods/guest/Cargo.toml

@@ -6,6 +6,8 @@ edition = "2021"
 [workspace]
 
 [dependencies]
-risc0-zkvm = { path = "../../../../risc0/zkvm", default-features = false, features = ["std"] }
 forust-ml = "0.4.2"
+risc0-zkvm = { path = "../../../../risc0/zkvm", default-features = false, features = ["std"] }
+# forust-ml uses rayon which expects to read RAYON_NUM_THREADS from the environment, so sys-getenv is enabled.
+risc0-zkvm-platform = { path = "../../../../risc0/zkvm/platform", default-features = false, features = ["sys-getenv"] }
 rmp-serde = "1.1.2"

license-check.py

@@ -32,6 +32,7 @@ SKIP_DIRS = [
     str(Path.cwd()) + "/risc0/sys/cxx/vendor",
     str(Path.cwd()) + "/risc0/zkvm/src/host/protos",
     str(Path.cwd()) + "/risc0/zkvm/src/host/server/exec",
+    str(Path.cwd()) + "/risc0/cargo-risczero/tests/test_crate",
 ]
 
 

risc0/build/src/docker.rs

@@ -265,7 +265,7 @@ mod test {
         build("../../risc0/zkvm/methods/guest/Cargo.toml");
         compare_image_id(
             "risc0_zkvm_methods_guest/hello_commit",
-            "f560fae4def57a6ac327b39817fd40a37fc5699cceb4a94ba75591933be53235",
+            "5e95ee67893f25399250181749cf26baab984b896c9ce04ac6e7cdf367e250f6",
         );
     }
 }

risc0/build/src/lib.rs

@@ -196,7 +196,7 @@ impl GuestBuilder for GuestListEntry {
             let image_id = match r0vm_image_id(elf_path) {
                 Ok(image_id) => image_id,
                 Err(err) => {
-                    tty_println(&format!("{err}"));
+                    tty_println(&format!("failed to get image ID using r0vm: {err}"));
                     compute_image_id(&elf)?
                 }
             };
@@ -486,12 +486,19 @@ fn cpp_toolchain_override() -> bool {
 
 /// Builds a static library providing a rust runtime.
 ///
-/// This can be used to build programs for the zkvm which don't depend on
-/// risc0_zkvm.
+/// This can be used to build programs for the zkvm which don't depend on risc0_zkvm.
 pub fn build_rust_runtime() -> String {
+    build_rust_runtime_with_features(&[])
+}
+
+/// Builds a static library providing a rust runtime, with additional features given as arguments.
+///
+/// This can be used to build programs for the zkvm which don't depend on risc0_zkvm. Feature flags
+/// given will be pass when building risc0-zkvm-platform.
+pub fn build_rust_runtime_with_features(features: &[&str]) -> String {
     build_staticlib(
         "risc0-zkvm-platform",
-        &["rust-runtime", "panic-handler", "entrypoint", "getrandom"],
+        &[&["rust-runtime", "panic-handler", "entrypoint"], features].concat(),
     )
 }
 

risc0/cargo-risczero/build.rs

@@ -16,16 +16,17 @@
 mod runtime {
     use std::{env, fs, io, path::Path};
 
-    use risc0_build::build_rust_runtime;
+    use risc0_build::build_rust_runtime_with_features;
     use zip::{write::SimpleFileOptions, CompressionMethod, ZipWriter};
 
-    pub fn build_and_zip_runtime() {
-        // Build the risc0-zkvm-platform.a file and place it in a zip archive for
-        // inclusion in the cargo-risczero binary.
+    pub(crate) fn build_and_zip_test_runtime() {
+        // Build the risc0-zkvm-platform.a file and place it in a zip archive for inclusion in the
+        // cargo-risczero binary. This is used to build test binaries for cargo risczero test
         let out_dir_env = env::var_os("OUT_DIR").unwrap();
         let out_dir = Path::new(&out_dir_env); // $ROOT/target/$profile/build/$crate/out
 
-        let rust_runtime = build_rust_runtime();
+        let rust_runtime =
+            build_rust_runtime_with_features(&["getrandom", "sys-getenv", "sys-args"]);
         let f = fs::File::create(out_dir.join("cargo-risczero.zip")).unwrap();
         let mut zip = ZipWriter::new(f);
         let options = SimpleFileOptions::default().compression_method(CompressionMethod::Stored);
@@ -43,6 +44,6 @@ fn main() {
         tracing_subscriber::fmt()
             .with_env_filter(tracing_subscriber::filter::EnvFilter::from_default_env())
             .init();
-        runtime::build_and_zip_runtime();
+        runtime::build_and_zip_test_runtime();
     }
 }

risc0/cargo-risczero/src/commands/build.rs

@@ -198,12 +198,19 @@ impl BuildCommand {
 
             for test in tests {
                 eprintln!("Running test in guest: {test} {test_args:?}");
-                let env = ExecutorEnv::builder()
+                let mut builder = ExecutorEnv::builder();
+                builder
                     // Add the test elf path as arg 0, the POSIX program name
                     .args(&[test.clone()])
                     .args(&test_args)
-                    .env_var("RUST_TEST_NOCAPTURE", "1")
-                    .build()?;
+                    .env_var("RUST_TEST_NOCAPTURE", "1");
+
+                // Forward all environment variables set on this process.
+                for (key, val) in std::env::vars().into_iter() {
+                    builder.env_var(&key, &val);
+                }
+
+                let env = builder.build()?;
 
                 let exec = default_executor();
                 let session = exec.execute(env, &fs::read(test)?)?;

risc0/cargo-risczero/tests/cargo_risczero_test.rs

@@ -0,0 +1,40 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#![cfg(feature = "experimental")]
+
+use std::process::Command;
+
+#[test]
+#[ignore = "failing as of oct 18 with with cargo 1.79"]
+fn basic_usage() {
+    let exe_path = env!("CARGO_BIN_EXE_cargo-risczero");
+    let args = ["risczero", "test"];
+    println!("{} {:?}", exe_path, args);
+    let output = Command::new(exe_path)
+        .env_clear()
+        .env("PATH", std::env::var("PATH").unwrap())
+        .env("RUST_TEST_THREADS", "1")
+        .current_dir("./tests/test_crate")
+        .args(args)
+        .output()
+        .unwrap();
+
+    println!("{:#?}", &output);
+
+    assert!(
+        output.status.success(),
+        "cargo risczero test command failed"
+    );
+}

risc0/cargo-risczero/tests/test_crate/.gitignore

@@ -0,0 +1,2 @@
+./target/
+!Cargo.lock

risc0/cargo-risczero/tests/test_crate/Cargo.lock

@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "cargo-risczero-test-crate"
+version = "0.1.0"

risc0/cargo-risczero/tests/test_crate/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "cargo-risczero-test-crate"
+version = "0.1.0"
+edition = "2021"
+
+[workspace]
+
+[dependencies]

risc0/cargo-risczero/tests/test_crate/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

risc0/zkvm/methods/Cargo.toml

@@ -11,7 +11,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 release = false
 
 [package.metadata.risc0]
-methods = ["cfg", "guest", "heap", "rand", "std", "cpp-crates"]
+methods = ["cfg", "guest", "heap", "rand", "std", "cpp-crates", "env"]
 
 [dependencies]
 bincode = { version = "1.3", optional = true }

risc0/zkvm/methods/env/.gitignore

@@ -0,0 +1 @@
+!Cargo.lock

risc0/zkvm/methods/env/Cargo.toml

@@ -0,0 +1,28 @@
+[workspace]
+
+# Without resolver = "2", it seems that sometimes features get enabled
+# in the guest based on features required by build dependencies.  If
+# resolver = "2" causes other problems, this may need to be
+# investigated further.
+resolver = "2"
+
+[package]
+name = "risc0-zkvm-methods-sys-env"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+# We don't enable getrandom, sys-getenv, or sys-args here.
+risc0-zkvm = { path = "../..", default-features = false, features = ["std"] }
+
+[profile.release]
+lto = "thin"
+opt-level = 3
+debug = 1
+
+[profile.release.package.risc0-zkvm-methods-sys-env]
+# Include debug symbols so we can test the profiler.
+debug = 1
+
+[package.metadata.release]
+release = false

risc0/zkvm/methods/env/src/bin/sys_args.rs

@@ -0,0 +1,22 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#![no_main]
+
+risc0_zkvm::guest::entry!(main);
+
+fn main() {
+    // Should panic, as sys_argc and sys_argv are disabled at build-time.
+    std::env::args().len();
+}

risc0/zkvm/methods/env/src/bin/sys_env.rs

@@ -0,0 +1,22 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#![no_main]
+
+risc0_zkvm::guest::entry!(main);
+
+fn main() {
+    // Should panic, as sys_getenv is disabled at build-time.
+    std::env::var("FOO").ok();
+}

risc0/zkvm/methods/std/Cargo.lock

@@ -1143,6 +1143,7 @@ version = "0.1.0"
 dependencies = [
  "risc0-zkvm",
  "risc0-zkvm-methods",
+ "risc0-zkvm-platform",
 ]
 
 [[package]]

risc0/zkvm/methods/std/Cargo.toml

@@ -17,6 +17,7 @@ risc0-zkvm = { path = "../..", default-features = false, features = [
   "getrandom",
 ] }
 risc0-zkvm-methods = { path = "..", features = ["std"] }
+risc0-zkvm-platform = { path = "../../platform", default-features = false, features = ["sys-getenv", "sys-args"] }
 
 [profile.release]
 lto = true

risc0/zkvm/platform/Cargo.toml

@@ -51,4 +51,8 @@ heap-embedded-alloc = [
 panic-handler = []
 # Build a rust runtime
 rust-runtime = ["export-libm", "export-syscalls"]
+# Enable the sys_getenv syscall.
+sys-getenv = []
+# Enable the sys_argc and sys_argv syscalls.
+sys-args = []
 unstable = []

risc0/zkvm/platform/src/syscall.rs

@@ -14,7 +14,7 @@
 
 #[cfg(target_os = "zkvm")]
 use core::arch::asm;
-use core::{cmp::min, ffi::CStr, ptr::null_mut, str::Utf8Error};
+use core::{cmp::min, ffi::CStr, ptr::null_mut, slice, str::Utf8Error};
 
 use crate::WORD_SIZE;
 
@@ -615,6 +615,11 @@ pub unsafe extern "C" fn sys_write(fd: u32, write_ptr: *const u8, nbytes: usize)
     }
 }
 
+// Some environment variable names are considered safe by default to use in the guest, provided by
+// the host, and are included in this list. It may be useful to allow guest developers to register
+// additional variable names as part of their guest program.
+const ALLOWED_ENV_VARNAMES: &[&[u8]] = &[b"RUST_BACKTRACE"];
+
 /// Retrieves the value of an environment variable, and stores as much
 /// of it as it can it in the memory at [out_words, out_words +
 /// out_nwords).
@@ -639,6 +644,23 @@ pub unsafe extern "C" fn sys_getenv(
     varname: *const u8,
     varname_len: usize,
 ) -> usize {
+    if cfg!(not(feature = "sys-getenv")) {
+        let mut allowed = false;
+        for allowed_varname in ALLOWED_ENV_VARNAMES {
+            let varname_buf = unsafe { slice::from_raw_parts(varname, varname_len) };
+            if *allowed_varname == varname_buf {
+                allowed = true;
+                break;
+            }
+        }
+        if !allowed {
+            const MSG_1: &[u8] = "sys_getenv not enabaled for var".as_bytes();
+            unsafe { sys_log(MSG_1.as_ptr(), MSG_1.len()) };
+            unsafe { sys_log(varname, varname_len) };
+            const MSG_2: &[u8] = "sys_getenv is disabled; can be enabled with the sys-getenv feature flag on risc0-zkvm-platform".as_bytes();
+            unsafe { sys_panic(MSG_2.as_ptr(), MSG_2.len()) };
+        }
+    }
     let Return(a0, _) = syscall_2(
         nr::SYS_GETENV,
         out_words,
@@ -659,6 +681,10 @@ pub unsafe extern "C" fn sys_getenv(
 /// data being returned. Returned data is entirely in the control of the host.
 #[cfg_attr(feature = "export-syscalls", no_mangle)]
 pub extern "C" fn sys_argc() -> usize {
+    if cfg!(not(feature = "sys-args")) {
+        const MSG: &[u8] = "sys_argc is disabled; can be enabled with the sys-args feature flag on risc0-zkvm-platform".as_bytes();
+        unsafe { sys_panic(MSG.as_ptr(), MSG.len()) };
+    }
     let Return(a0, _) = unsafe { syscall_0(nr::SYS_ARGC, null_mut(), 0) };
     a0 as usize
 }
@@ -686,6 +712,10 @@ pub unsafe extern "C" fn sys_argv(
     out_nwords: usize,
     arg_index: usize,
 ) -> usize {
+    if cfg!(not(feature = "sys-args")) {
+        const MSG: &[u8] = "sys_argv is disabled; can be enabled with the sys-args feature flag on risc0-zkvm-platform".as_bytes();
+        unsafe { sys_panic(MSG.as_ptr(), MSG.len()) };
+    }
     let Return(a0, _) = syscall_1(nr::SYS_ARGV, out_words, out_nwords, arg_index as u32);
     a0 as usize
 }

risc0/zkvm/src/host/server/exec/tests.rs

@@ -25,7 +25,7 @@ use risc0_binfmt::{MemoryImage, Program};
 use risc0_zkvm_methods::{
     multi_test::{MultiTestSpec, SYS_MULTI_TEST, SYS_MULTI_TEST_WORDS},
     BLST_ELF, HEAP_ELF, HELLO_COMMIT_ELF, MULTI_TEST_ELF, RAND_ELF, SLICE_IO_ELF, STANDARD_LIB_ELF,
-    ZKVM_527_ELF,
+    SYS_ARGS_ELF, SYS_ENV_ELF, ZKVM_527_ELF,
 };
 use risc0_zkvm_platform::{fileno, syscall::nr::SYS_RANDOM, PAGE_SIZE, WORD_SIZE};
 use sha2::{Digest as _, Sha256};
@@ -875,6 +875,26 @@ fn getrandom_panic() {
         .unwrap();
 }
 
+#[test]
+#[should_panic(expected = "Guest panicked: sys_getenv is disabled")]
+fn sys_getenv_panic() {
+    let env = ExecutorEnv::builder().build().unwrap();
+    let _session = ExecutorImpl::from_elf(env, SYS_ENV_ELF)
+        .unwrap()
+        .run()
+        .unwrap();
+}
+
+#[test]
+#[should_panic(expected = "Guest panicked: sys_argc is disabled")]
+fn sys_args_panic() {
+    let env = ExecutorEnv::builder().build().unwrap();
+    let _session = ExecutorImpl::from_elf(env, SYS_ARGS_ELF)
+        .unwrap()
+        .run()
+        .unwrap();
+}
+
 #[test]
 fn slice_io() {
     let run = |slice: &[u8]| {