From f22bc2428ff5930b9d3d66d5983e22147988c4a1 Mon Sep 17 00:00:00 2001
From: Marcin S <marcin@realemail.net>
Date: Mon, 21 Aug 2023 15:18:31 +0200
Subject: [PATCH] PVF worker: Prevent access to env vars (#7330)

---
 polkadot/node/core/pvf/common/src/worker/mod.rs        | 10 ++++++++++
 .../src/node/utility/pvf-host-and-workers.md           |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/polkadot/node/core/pvf/common/src/worker/mod.rs b/polkadot/node/core/pvf/common/src/worker/mod.rs
index d9a0dff71b..d249007ec3 100644
--- a/polkadot/node/core/pvf/common/src/worker/mod.rs
+++ b/polkadot/node/core/pvf/common/src/worker/mod.rs
@@ -128,6 +128,16 @@ pub fn worker_event_loop<F, Fut>(
 		}
 	}
 
+	// Delete all env vars to prevent malicious code from accessing them.
+	for (key, _) in std::env::vars() {
+		// TODO: *theoretically* the value (or mere presence) of `RUST_LOG` can be a source of
+		// randomness for malicious code. In the future we can remove it also and log in the host;
+		// see <https://github.com/paritytech/polkadot/issues/7117>.
+		if key != "RUST_LOG" {
+			std::env::remove_var(key);
+		}
+	}
+
 	// Run the main worker loop.
 	let rt = Runtime::new().expect("Creates tokio runtime. If this panics the worker will die and the host will detect that and deal with it.");
 	let err = rt
diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md
index 017b7fc025..bcf01b61f2 100644
--- a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md
+++ b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md
@@ -125,3 +125,10 @@ A basic security mechanism is to make sure that any thread directly interfacing
 with untrusted code does not have access to the file-system. This provides some
 protection against attackers accessing sensitive data or modifying data on the
 host machine.
+
+### Clearing env vars
+
+We clear environment variables before handling untrusted code, because why give
+attackers potentially sensitive data unnecessarily? And even if everything else
+is locked down, env vars can potentially provide a source of randomness (see
+point 1, "Consensus faults" above).