From 3386753b48db3c7d4e79d41697cb6b6454db33c8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:47:37 +0000 Subject: [PATCH 1/6] Reject nulls in process environment names Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Resources/Strings.resx | 3 +++ .../Collections/Specialized/DictionaryWrapper.cs | 14 +++++++++++++- .../tests/ProcessStartInfoTests.cs | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index b1aa722a5ddedc..e8a605efa4ca92 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -318,6 +318,9 @@ The output char buffer is too small to contain the decoded characters, encoding '{0}' fallback '{1}'. + + Environment variable name cannot contain a null character. + Too many characters. The resulting number of bytes is larger than what can be returned as an int. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs index f8d53afbb659f5..7cfc22bb615ef2 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs @@ -17,7 +17,11 @@ public DictionaryWrapper(Dictionary contents) public string? this[string key] { get => _contents[key]; - set => _contents[key] = value; + set + { + ValidateKey(key); + _contents[key] = value; + } } public object? this[object key] @@ -81,5 +85,13 @@ public bool Remove(KeyValuePair item) public IEnumerator> GetEnumerator() => _contents.GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => _contents.GetEnumerator(); IDictionaryEnumerator IDictionary.GetEnumerator() => _contents.GetEnumerator(); + + private static void ValidateKey(string key) + { + if (key.Contains('\0')) + { + throw new ArgumentException(SR.Argument_EnvironmentVariableNameContainsNull, nameof(key)); + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index ba1eb8445aa33c..96de90338dbea6 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -210,6 +210,21 @@ public void TestEnvironmentProperty() }); } + [Fact] + public void EnvironmentVariableNameContainingNull_ThrowsArgumentException() + { + const string InvalidName = "Name\0Suffix"; + ProcessStartInfo psi = new ProcessStartInfo(); + IDictionary environment = (IDictionary)psi.Environment; + ICollection> environmentCollection = psi.Environment; + + AssertExtensions.Throws("key", () => psi.Environment[InvalidName] = "value"); + AssertExtensions.Throws("key", () => environment[InvalidName] = "value"); + AssertExtensions.Throws("key", () => psi.Environment.Add(InvalidName, "value")); + AssertExtensions.Throws("key", () => environmentCollection.Add(new KeyValuePair(InvalidName, "value"))); + AssertExtensions.Throws("key", () => environment.Add(InvalidName, "value")); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void TestSetEnvironmentOnChildProcess() { From 9b8b6db4232fde062c97309039e2ab45a625fb37 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:53:41 +0000 Subject: [PATCH 2/6] Preserve null key validation Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Collections/Specialized/DictionaryWrapper.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs index 7cfc22bb615ef2..263554dfbafa2f 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs @@ -88,6 +88,8 @@ public bool Remove(KeyValuePair item) private static void ValidateKey(string key) { + ArgumentNullException.ThrowIfNull(key); + if (key.Contains('\0')) { throw new ArgumentException(SR.Argument_EnvironmentVariableNameContainsNull, nameof(key)); From 327a2ca12db00845f860417e46216a70f13ea770 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:24:58 +0000 Subject: [PATCH 3/6] Reject nulls in environment variable values Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Resources/Strings.resx | 4 ++-- .../Specialized/DictionaryWrapper.cs | 14 ++++++++++--- .../tests/ProcessStartInfoTests.cs | 21 ++++++++++++------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index e8a605efa4ca92..df17778b76ff0a 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -318,8 +318,8 @@ The output char buffer is too small to contain the decoded characters, encoding '{0}' fallback '{1}'. - - Environment variable name cannot contain a null character. + + Environment variable cannot contain a null character. Too many characters. The resulting number of bytes is larger than what can be returned as an int. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs index 263554dfbafa2f..a84213a12f276b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs @@ -20,6 +20,7 @@ public string? this[string key] set { ValidateKey(key); + ValidateValue(value); _contents[key] = value; } } @@ -88,12 +89,19 @@ public bool Remove(KeyValuePair item) private static void ValidateKey(string key) { - ArgumentNullException.ThrowIfNull(key); + if (key is not null && key.Contains('\0')) + { + throw new ArgumentException(SR.Argument_NullCharInEnvVar, nameof(key)); + } + } - if (key.Contains('\0')) + private static void ValidateValue(string? value) + { + if (value is not null && value.Contains('\0')) { - throw new ArgumentException(SR.Argument_EnvironmentVariableNameContainsNull, nameof(key)); + throw new ArgumentException(SR.Argument_NullCharInEnvVar, nameof(value)); } } + } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 96de90338dbea6..a187c49c733c4b 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -211,18 +211,25 @@ public void TestEnvironmentProperty() } [Fact] - public void EnvironmentVariableNameContainingNull_ThrowsArgumentException() + public void EnvironmentVariableContainingNull_ThrowsArgumentException() { - const string InvalidName = "Name\0Suffix"; + const string InvalidKey = "Name\0Suffix"; + const string InvalidValue = "Value\0Suffix"; ProcessStartInfo psi = new ProcessStartInfo(); IDictionary environment = (IDictionary)psi.Environment; ICollection> environmentCollection = psi.Environment; - AssertExtensions.Throws("key", () => psi.Environment[InvalidName] = "value"); - AssertExtensions.Throws("key", () => environment[InvalidName] = "value"); - AssertExtensions.Throws("key", () => psi.Environment.Add(InvalidName, "value")); - AssertExtensions.Throws("key", () => environmentCollection.Add(new KeyValuePair(InvalidName, "value"))); - AssertExtensions.Throws("key", () => environment.Add(InvalidName, "value")); + AssertExtensions.Throws("key", () => psi.Environment[InvalidKey] = "value"); + AssertExtensions.Throws("key", () => environment[InvalidKey] = "value"); + AssertExtensions.Throws("key", () => psi.Environment.Add(InvalidKey, "value")); + AssertExtensions.Throws("key", () => environmentCollection.Add(new KeyValuePair(InvalidKey, "value"))); + AssertExtensions.Throws("key", () => environment.Add(InvalidKey, "value")); + + AssertExtensions.Throws("value", () => psi.Environment["key"] = InvalidValue); + AssertExtensions.Throws("value", () => environment["key"] = InvalidValue); + AssertExtensions.Throws("value", () => psi.Environment.Add("key", InvalidValue)); + AssertExtensions.Throws("value", () => environmentCollection.Add(new KeyValuePair("key", InvalidValue))); + AssertExtensions.Throws("value", () => environment.Add("key", InvalidValue)); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] From dedf422c2659268c57fdacc9f40a274153021cd8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:36:45 +0000 Subject: [PATCH 4/6] Reduce env-var null-char validation duplication Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Collections/Specialized/DictionaryWrapper.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs index a84213a12f276b..fe21dc780a31d4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs @@ -19,8 +19,8 @@ public string? this[string key] get => _contents[key]; set { - ValidateKey(key); - ValidateValue(value); + Validate(nameof(key), key); + Validate(nameof(value), value); _contents[key] = value; } } @@ -87,19 +87,11 @@ public bool Remove(KeyValuePair item) IEnumerator IEnumerable.GetEnumerator() => _contents.GetEnumerator(); IDictionaryEnumerator IDictionary.GetEnumerator() => _contents.GetEnumerator(); - private static void ValidateKey(string key) - { - if (key is not null && key.Contains('\0')) - { - throw new ArgumentException(SR.Argument_NullCharInEnvVar, nameof(key)); - } - } - - private static void ValidateValue(string? value) + private static void Validate(string name, string? value) { if (value is not null && value.Contains('\0')) { - throw new ArgumentException(SR.Argument_NullCharInEnvVar, nameof(value)); + throw new ArgumentException(SR.Argument_NullCharInEnvVar, name); } } From 0f3209191ad52d9920bb7dc47b483d3a87b790cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:24:49 +0000 Subject: [PATCH 5/6] Document environment null character validation Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessUtils.Windows.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs index b2dc5a8d8828d7..63a9637d88433e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Windows.cs @@ -110,6 +110,7 @@ internal static string GetEnvironmentVariablesBlock(DictionaryWrapper sd) // Ignore null values for consistency with Environment.SetEnvironmentVariable if (value != null) { + // DictionaryWrapper prevents user-supplied keys and values from containing null characters. result.Append(key).Append('=').Append(value).Append('\0'); } } From dae8cfea04704ce8a8adccca38008087f7c802c1 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 13 Aug 2026 11:27:54 +0200 Subject: [PATCH 6/6] Update src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs --- .../src/System/Collections/Specialized/DictionaryWrapper.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs index fe21dc780a31d4..c938def6643b4a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Collections/Specialized/DictionaryWrapper.cs @@ -21,6 +21,7 @@ public string? this[string key] { Validate(nameof(key), key); Validate(nameof(value), value); + _contents[key] = value; } }