From 36eaab2809b4cf7ad5bfa8cabd8b29016f0db84d Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 6 Jul 2017 09:09:57 -0700 Subject: [PATCH] Static analysis: Fix usage of XmlReader to ensure XmlResolver is null (#4108) * the default XmlResolver will attempt to resolve external resources, recommendation is to explicitly set to null which will raise exception if malicious xml attempts to cause xmlreader to access external resources * added DTDProcessing and related to XmlReaderSettings --- .../Common/InvokeRestMethodCommand.Common.cs | 1 + .../host/msh/Serialization.cs | 2 +- .../security/CertificateProvider.cs | 2 -- .../WorkflowCore/WorkflowRuntimeCompilation.cs | 2 +- .../engine/TypeTable.cs | 2 +- .../remoting/fanin/OutOfProcTransportManager.cs | 2 -- .../remoting/fanin/PSSessionConfigurationData.cs | 2 -- .../engine/serialization.cs | 15 +++------------ 8 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs index 0559e35daa..90abdf78bb 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs @@ -133,6 +133,7 @@ namespace Microsoft.PowerShell.Commands xrs.IgnoreProcessingInstructions = true; xrs.MaxCharactersFromEntities = 1024; xrs.DtdProcessing = DtdProcessing.Ignore; + xrs.XmlResolver = null; return xrs; } diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/Serialization.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/Serialization.cs index abcd4ff2cf..f73c26107f 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/Serialization.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/Serialization.cs @@ -197,7 +197,7 @@ namespace Microsoft.PowerShell switch (format) { case DataFormat.XML: - _xmlReader = XmlReader.Create(textReader); + _xmlReader = XmlReader.Create(textReader, new XmlReaderSettings { XmlResolver = null }); _xmlDeserializer = new Deserializer(_xmlReader); break; case DataFormat.Text: diff --git a/src/Microsoft.PowerShell.Security/security/CertificateProvider.cs b/src/Microsoft.PowerShell.Security/security/CertificateProvider.cs index b8bcee8bd4..dc84f9a3b4 100644 --- a/src/Microsoft.PowerShell.Security/security/CertificateProvider.cs +++ b/src/Microsoft.PowerShell.Security/security/CertificateProvider.cs @@ -2876,9 +2876,7 @@ namespace Microsoft.PowerShell.Commands currentUICulture.ToString(), this.ProviderInfo.HelpFile); XmlReaderSettings settings = new XmlReaderSettings(); -#if !CORECLR settings.XmlResolver = null; -#endif using (XmlReader reader = XmlReader.Create(fullHelpPath, settings)) { document.Load(reader); diff --git a/src/Microsoft.PowerShell.Workflow.ServiceCore/ServiceCore/WorkflowCore/WorkflowRuntimeCompilation.cs b/src/Microsoft.PowerShell.Workflow.ServiceCore/ServiceCore/WorkflowCore/WorkflowRuntimeCompilation.cs index 2ef5dce6d6..0955fa66e6 100644 --- a/src/Microsoft.PowerShell.Workflow.ServiceCore/ServiceCore/WorkflowCore/WorkflowRuntimeCompilation.cs +++ b/src/Microsoft.PowerShell.Workflow.ServiceCore/ServiceCore/WorkflowCore/WorkflowRuntimeCompilation.cs @@ -135,7 +135,7 @@ namespace Microsoft.PowerShell.Workflow { project.AddItem("XamlAppDef", file); - XamlXmlReader reader = new XamlXmlReader(XmlReader.Create(file), new XamlSchemaContext()); + XamlXmlReader reader = new XamlXmlReader(XmlReader.Create(file, new XmlReaderSettings { XmlResolver = null }), new XamlSchemaContext()); using (reader) { while (reader.Read()) diff --git a/src/System.Management.Automation/engine/TypeTable.cs b/src/System.Management.Automation/engine/TypeTable.cs index 41ae3c93f9..57ded79b28 100644 --- a/src/System.Management.Automation/engine/TypeTable.cs +++ b/src/System.Management.Automation/engine/TypeTable.cs @@ -4178,7 +4178,7 @@ namespace System.Management.Automation.Runspaces { #if CORECLR // In OneCore powershell, XmlTextReader is not in CoreCLR, so we have to use XmlReader.Create method - XmlReader reader = XmlReader.Create(xmlStream, new XmlReaderSettings { IgnoreWhitespace = true }); + XmlReader reader = XmlReader.Create(xmlStream, new XmlReaderSettings { IgnoreWhitespace = true, XmlResolver = null }); #else // In Full powershell, we create a XmlTextReader, so loadContext.reader is guaranteed to implement IXmlLineInfo XmlReader reader = new XmlTextReader(xmlStream) { WhitespaceHandling = WhitespaceHandling.Significant }; diff --git a/src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs b/src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs index 90a5be4cf0..be46f4532d 100644 --- a/src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs +++ b/src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs @@ -65,9 +65,7 @@ namespace System.Management.Automation.Remoting XmlReaderSettings.CheckCharacters = false; XmlReaderSettings.IgnoreComments = true; XmlReaderSettings.IgnoreProcessingInstructions = true; -#if !CORECLR // No XmlReaderSettings.XmlResolver in CoreCLR XmlReaderSettings.XmlResolver = null; -#endif XmlReaderSettings.ConformanceLevel = ConformanceLevel.Fragment; } diff --git a/src/System.Management.Automation/engine/remoting/fanin/PSSessionConfigurationData.cs b/src/System.Management.Automation/engine/remoting/fanin/PSSessionConfigurationData.cs index 4b11b4472d..fd480daf58 100644 --- a/src/System.Management.Automation/engine/remoting/fanin/PSSessionConfigurationData.cs +++ b/src/System.Management.Automation/engine/remoting/fanin/PSSessionConfigurationData.cs @@ -89,9 +89,7 @@ namespace System.Management.Automation.Remoting IgnoreComments = true, IgnoreProcessingInstructions = true, MaxCharactersInDocument = 10000, -#if !CORECLR // No XmlReaderSettings.XmlResolver in CoreCLR XmlResolver = null, -#endif ConformanceLevel = ConformanceLevel.Fragment }; diff --git a/src/System.Management.Automation/engine/serialization.cs b/src/System.Management.Automation/engine/serialization.cs index b1a223bf00..716b8eb695 100644 --- a/src/System.Management.Automation/engine/serialization.cs +++ b/src/System.Management.Automation/engine/serialization.cs @@ -4026,15 +4026,11 @@ namespace System.Management.Automation xrs.IgnoreProcessingInstructions = true; xrs.IgnoreWhitespace = false; xrs.MaxCharactersFromEntities = 1024; - //xrs.DtdProcessing = DtdProcessing.Prohibit; //because system.management.automation needs to build as 2.0 - //xrs.ProhibitDtd = true; -#if !CORECLR - // XmlReaderSettings.Schemas/ValidationFlags/ValidationType/XmlResolver Not In CoreCLR + xrs.XmlResolver = null; + xrs.DtdProcessing = DtdProcessing.Prohibit; xrs.Schemas = null; xrs.ValidationFlags = System.Xml.Schema.XmlSchemaValidationFlags.None; xrs.ValidationType = ValidationType.None; - xrs.XmlResolver = null; -#endif return xrs; } @@ -4051,15 +4047,10 @@ namespace System.Management.Automation settings.IgnoreWhitespace = true; settings.MaxCharactersFromEntities = 1024; settings.MaxCharactersInDocument = 512 * 1024 * 1024; // 512M characters = 1GB - -#if CORECLR // DtdProcessing.Parse Not In CoreCLR - settings.DtdProcessing = DtdProcessing.Ignore; -#else // XmlReaderSettings.ValidationFlags/ValidationType/XmlResolver Not In CoreCLR + settings.XmlResolver = null; settings.DtdProcessing = DtdProcessing.Parse; // Allowing DTD parsing with limits of MaxCharactersFromEntities/MaxCharactersInDocument settings.ValidationFlags = System.Xml.Schema.XmlSchemaValidationFlags.None; settings.ValidationType = ValidationType.None; - settings.XmlResolver = null; -#endif return settings; }