diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index b1aa722a5ddedc..df17778b76ff0a 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 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..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 @@ -17,7 +17,13 @@ public DictionaryWrapper(Dictionary contents) public string? this[string key] { get => _contents[key]; - set => _contents[key] = value; + set + { + Validate(nameof(key), key); + Validate(nameof(value), value); + + _contents[key] = value; + } } public object? this[object key] @@ -81,5 +87,14 @@ public bool Remove(KeyValuePair item) public IEnumerator> GetEnumerator() => _contents.GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => _contents.GetEnumerator(); IDictionaryEnumerator IDictionary.GetEnumerator() => _contents.GetEnumerator(); + + private static void Validate(string name, string? value) + { + if (value is not null && value.Contains('\0')) + { + throw new ArgumentException(SR.Argument_NullCharInEnvVar, name); + } + } + } } 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'); } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index ba1eb8445aa33c..a187c49c733c4b 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -210,6 +210,28 @@ public void TestEnvironmentProperty() }); } + [Fact] + public void EnvironmentVariableContainingNull_ThrowsArgumentException() + { + 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[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))] public void TestSetEnvironmentOnChildProcess() {