From 8d59958cd20d5f6881c4e7a4cbc3503198398b0e Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 19 Jul 2023 09:26:45 -0700 Subject: [PATCH 1/4] clarify difference between violation/solution --- .../code-analysis/quality-rules/ca3009.md | 142 +++--------------- .../snippets/csharp/all-rules/ca3009.cs | 51 +++++++ .../snippets/vb/all-rules/ca3009.vb | 51 +++++++ 3 files changed, 125 insertions(+), 119 deletions(-) create mode 100644 docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca3009.cs create mode 100644 docs/fundamentals/code-analysis/quality-rules/snippets/vb/all-rules/ca3009.vb diff --git a/docs/fundamentals/code-analysis/quality-rules/ca3009.md b/docs/fundamentals/code-analysis/quality-rules/ca3009.md index 22194d24ff3a5..f9dc5457a1401 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca3009.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca3009.md @@ -1,7 +1,7 @@ --- title: "CA3009: Review code for XML injection vulnerabilities (code analysis)" description: "Learn about code analysis rule CA3009: Review code for XML injection vulnerabilities" -ms.date: 07/21/2020 +ms.date: 07/19/2023 ms.topic: reference author: dotpaul ms.author: paulming @@ -38,16 +38,34 @@ This rule attempts to find input from HTTP requests reaching a raw XML write. ## How to fix violations -Don't write raw XML. Instead, use methods or properties that XML-encode their input. +To fix a violation, use one of the following techniques: -Or, XML-encode input before writing raw XML. - -Or, validate user input by using sanitizers for primitive type conversion and XML encoding. +- Don't write raw XML. Instead, use methods or properties that XML-encode their input. +- XML-encode input before writing raw XML. +- Validate user input by using sanitizers for primitive type conversion and XML encoding. ## When to suppress warnings Don't suppress warnings from this rule. +## Pseudo-code examples + +### Violation + +In this example, the input is set to the property of the root element. Given input that contains valid XML, a malicious user can then completely alter the document. Notice that `alice` is no longer an allowed user after the user input is added to the document. + +:::code language="csharp" source="snippets/ca3009.cs" id="violation" highlight=19::: + +:::code language="vb" source="snippets/ca3009.vb" id="violation" highlight=19::: + +### Solution + +To fix this violation, set the input to the property of the root element instead of the property. + +:::code language="csharp" source="snippets/ca3009.cs" id="fix" highlight=43::: + +:::code language="vb" source="snippets/ca3009.vb" id="fix" highlight=43::: + ## Configure code to analyze Use the following options to configure which parts of your codebase to run this rule on. @@ -60,117 +78,3 @@ You can configure these options for just this rule, for all rules it applies to, [!INCLUDE[excluded-symbol-names](../includes/excluded-symbol-names.md)] [!INCLUDE[excluded-type-names-with-derived-types](../includes/excluded-type-names-with-derived-types.md)] - -## Pseudo-code examples - -### Violation - -```csharp -using System; -using System.Xml; - -public partial class WebForm : System.Web.UI.Page -{ - protected void Page_Load(object sender, EventArgs e) - { - string input = Request.Form["in"]; - XmlDocument d = new XmlDocument(); - XmlElement root = d.CreateElement("root"); - d.AppendChild(root); - - XmlElement allowedUser = d.CreateElement("allowedUser"); - root.AppendChild(allowedUser); - - allowedUser.InnerXml = "alice"; - - // If an attacker uses this for input: - // some textoscar - // Then the XML document will be: - // some textoscar - root.InnerXml = input; - } -} -``` - -```vb -Imports System -Imports System.Xml - -Public Partial Class WebForm - Inherits System.Web.UI.Page - - Sub Page_Load(sender As Object, e As EventArgs) - Dim input As String = Request.Form("in") - Dim d As XmlDocument = New XmlDocument() - Dim root As XmlElement = d.CreateElement("root") - d.AppendChild(root) - - Dim allowedUser As XmlElement = d.CreateElement("allowedUser") - root.AppendChild(allowedUser) - - allowedUser.InnerXml = "alice" - - ' If an attacker uses this for input: - ' some textoscar - ' Then the XML document will be: - ' some textoscar - root.InnerXml = input - End Sub -End Class -``` - -### Solution - -```csharp -using System; -using System.Xml; - -public partial class WebForm : System.Web.UI.Page -{ - protected void Page_Load(object sender, EventArgs e) - { - string input = Request.Form["in"]; - XmlDocument d = new XmlDocument(); - XmlElement root = d.CreateElement("root"); - d.AppendChild(root); - - XmlElement allowedUser = d.CreateElement("allowedUser"); - root.AppendChild(allowedUser); - - allowedUser.InnerText = "alice"; - - // If an attacker uses this for input: - // some textoscar - // Then the XML document will be: - // <allowedUser>oscar</allowedUser>some textalice - root.InnerText = input; - } -} -``` - -```vb -Imports System -Imports System.Xml - -Public Partial Class WebForm - Inherits System.Web.UI.Page - - Sub Page_Load(sender As Object, e As EventArgs) - Dim input As String = Request.Form("in") - Dim d As XmlDocument = New XmlDocument() - Dim root As XmlElement = d.CreateElement("root") - d.AppendChild(root) - - Dim allowedUser As XmlElement = d.CreateElement("allowedUser") - root.AppendChild(allowedUser) - - allowedUser.InnerText = "alice" - - ' If an attacker uses this for input: - ' some textoscar - ' Then the XML document will be: - ' <allowedUser>oscar</allowedUser>some textalice - root.InnerText = input - End Sub -End Class -``` diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca3009.cs b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca3009.cs new file mode 100644 index 0000000000000..40e6d1c3b2f4e --- /dev/null +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca3009.cs @@ -0,0 +1,51 @@ +using System; +using System.Xml; + +public partial class WebForm1 : System.Web.UI.Page +{ + // + protected void Page_Load(object sender, EventArgs e) + { + + XmlDocument d = new XmlDocument(); + XmlElement root = d.CreateElement("root"); + d.AppendChild(root); + + XmlElement allowedUser = d.CreateElement("allowedUser"); + root.AppendChild(allowedUser); + allowedUser.InnerXml = "alice"; + + string input = Request.Form["in"]; + root.InnerXml = input; + + // If an attacker uses this for input: + // some textoscar + // Then the XML document will be: + // some textoscar + } + // +} + +public partial class WebForm2 : System.Web.UI.Page +{ + // + protected void Page_Load(object sender, EventArgs e) + { + XmlDocument d = new XmlDocument(); + XmlElement root = d.CreateElement("root"); + d.AppendChild(root); + + XmlElement allowedUser = d.CreateElement("allowedUser"); + root.AppendChild(allowedUser); + allowedUser.InnerText = "alice"; + + string input = Request.Form["in"]; + root.InnerText = input; + + // If an attacker uses this for input: + // some textoscar + // Then the XML document will be: + // <allowedUser>oscar</allowedUser>some textalice + } + // +} diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/vb/all-rules/ca3009.vb b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/all-rules/ca3009.vb new file mode 100644 index 0000000000000..ea0ff79c36f8d --- /dev/null +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/all-rules/ca3009.vb @@ -0,0 +1,51 @@ + +Imports System +Imports System.Xml + +Public Partial Class WebForm1 + Inherits System.Web.UI.Page + + ' + Sub Page_Load(sender As Object, e As EventArgs) + Dim d As XmlDocument = New XmlDocument() + Dim root As XmlElement = d.CreateElement("root") + d.AppendChild(root) + + Dim allowedUser As XmlElement = d.CreateElement("allowedUser") + root.AppendChild(allowedUser) + allowedUser.InnerXml = "alice" + + Dim input As String = Request.Form("in") + root.InnerXml = input + + ' If an attacker uses this for input: + ' some textoscar + ' Then the XML document will be: + ' some textoscar + End Sub + ' +End Class + +Public Partial Class WebForm2 + Inherits System.Web.UI.Page + + ' + Sub Page_Load(sender As Object, e As EventArgs) + Dim d As XmlDocument = New XmlDocument() + Dim root As XmlElement = d.CreateElement("root") + d.AppendChild(root) + + Dim allowedUser As XmlElement = d.CreateElement("allowedUser") + root.AppendChild(allowedUser) + allowedUser.InnerText = "alice" + + Dim input As String = Request.Form("in") + root.InnerText = input + + ' If an attacker uses this for input: + ' some textoscar + ' Then the XML document will be: + ' <allowedUser>oscar</allowedUser>some textalice + End Sub + ' +End Class From 7970f09379abef109552c058e32ac180f737d0c8 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:24:48 -0700 Subject: [PATCH 2/4] move to .net framework project --- .../code-analysis/quality-rules/ca3009.md | 8 ++++---- .../snippets/csharp/{all-rules => netfx}/ca3009.cs | 0 .../snippets/csharp/netfx/project.csproj | 12 ++++++++++++ .../snippets/vb/{all-rules => netfx}/ca3009.vb | 0 .../quality-rules/snippets/vb/netfx/project.vbproj | 12 ++++++++++++ 5 files changed, 28 insertions(+), 4 deletions(-) rename docs/fundamentals/code-analysis/quality-rules/snippets/csharp/{all-rules => netfx}/ca3009.cs (100%) create mode 100644 docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/project.csproj rename docs/fundamentals/code-analysis/quality-rules/snippets/vb/{all-rules => netfx}/ca3009.vb (100%) create mode 100644 docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/project.vbproj diff --git a/docs/fundamentals/code-analysis/quality-rules/ca3009.md b/docs/fundamentals/code-analysis/quality-rules/ca3009.md index f9dc5457a1401..0ac30f8989873 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca3009.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca3009.md @@ -54,17 +54,17 @@ Don't suppress warnings from this rule. In this example, the input is set to the property of the root element. Given input that contains valid XML, a malicious user can then completely alter the document. Notice that `alice` is no longer an allowed user after the user input is added to the document. -:::code language="csharp" source="snippets/ca3009.cs" id="violation" highlight=19::: +:::code language="csharp" source="snippets/csharp/netfx/ca3009.cs" id="violation" highlight="19"::: -:::code language="vb" source="snippets/ca3009.vb" id="violation" highlight=19::: +:::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="violation" highlight="19"::: ### Solution To fix this violation, set the input to the property of the root element instead of the property. -:::code language="csharp" source="snippets/ca3009.cs" id="fix" highlight=43::: +:::code language="csharp" source="snippets/csharp/netfx/ca3009.cs" id="fix" highlight="43"::: -:::code language="vb" source="snippets/ca3009.vb" id="fix" highlight=43::: +:::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="fix" highlight="43"::: ## Configure code to analyze diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca3009.cs b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs similarity index 100% rename from docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca3009.cs rename to docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/project.csproj b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/project.csproj new file mode 100644 index 0000000000000..cdba31d53c4b9 --- /dev/null +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/project.csproj @@ -0,0 +1,12 @@ + + + + Library + net48 + + + + + + + diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/vb/all-rules/ca3009.vb b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/ca3009.vb similarity index 100% rename from docs/fundamentals/code-analysis/quality-rules/snippets/vb/all-rules/ca3009.vb rename to docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/ca3009.vb diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/project.vbproj b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/project.vbproj new file mode 100644 index 0000000000000..1216668729024 --- /dev/null +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/project.vbproj @@ -0,0 +1,12 @@ + + + + Library + net48 + + + + + + + From 96375acffef72974b403148aac1d78b0a1fce3c9 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:39:19 -0700 Subject: [PATCH 3/4] fix highlights --- docs/fundamentals/code-analysis/quality-rules/ca3009.md | 8 ++++---- .../quality-rules/snippets/csharp/netfx/ca3009.cs | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca3009.md b/docs/fundamentals/code-analysis/quality-rules/ca3009.md index 0ac30f8989873..8f0e608ca271d 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca3009.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca3009.md @@ -54,17 +54,17 @@ Don't suppress warnings from this rule. In this example, the input is set to the property of the root element. Given input that contains valid XML, a malicious user can then completely alter the document. Notice that `alice` is no longer an allowed user after the user input is added to the document. -:::code language="csharp" source="snippets/csharp/netfx/ca3009.cs" id="violation" highlight="19"::: +:::code language="csharp" source="snippets/csharp/netfx/ca3009.cs" id="violation" highlight="12"::: -:::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="violation" highlight="19"::: +:::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="violation" highlight="11"::: ### Solution To fix this violation, set the input to the property of the root element instead of the property. -:::code language="csharp" source="snippets/csharp/netfx/ca3009.cs" id="fix" highlight="43"::: +:::code language="csharp" source="snippets/csharp/netfx/ca3009.cs" id="fix" highlight="12"::: -:::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="fix" highlight="43"::: +:::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="fix" highlight="11"::: ## Configure code to analyze diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs index 40e6d1c3b2f4e..25de68d5ab9a5 100644 --- a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs @@ -6,7 +6,6 @@ public partial class WebForm1 : System.Web.UI.Page // protected void Page_Load(object sender, EventArgs e) { - XmlDocument d = new XmlDocument(); XmlElement root = d.CreateElement("root"); d.AppendChild(root); From 221f0369136c9f0e71f25786d691662445c8439e Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 19 Jul 2023 10:49:51 -0700 Subject: [PATCH 4/4] fix rendering of angle brackets --- .../code-analysis/quality-rules/ca3009.md | 15 +++++++++++++++ .../quality-rules/snippets/csharp/netfx/ca3009.cs | 10 ---------- .../quality-rules/snippets/vb/netfx/ca3009.vb | 10 ---------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca3009.md b/docs/fundamentals/code-analysis/quality-rules/ca3009.md index 8f0e608ca271d..2204d6937c1c3 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca3009.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca3009.md @@ -58,6 +58,13 @@ In this example, the input is set to the p :::code language="vb" source="snippets/vb/netfx/ca3009.vb" id="violation" highlight="11"::: +If an attacker uses this for input: `some textoscar`, then the XML document will be: + +```xml +some textoscar + +``` + ### Solution To fix this violation, set the input to the property of the root element instead of the property. @@ -66,6 +73,14 @@ To fix this violation, set the input to the oscar`, then the XML document will be: + +```xml +some text<allowedUser>oscar</allowedUser> +alice + +``` + ## Configure code to analyze Use the following options to configure which parts of your codebase to run this rule on. diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs index 25de68d5ab9a5..9a97376520eee 100644 --- a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/netfx/ca3009.cs @@ -16,11 +16,6 @@ protected void Page_Load(object sender, EventArgs e) string input = Request.Form["in"]; root.InnerXml = input; - - // If an attacker uses this for input: - // some textoscar - // Then the XML document will be: - // some textoscar } // } @@ -40,11 +35,6 @@ protected void Page_Load(object sender, EventArgs e) string input = Request.Form["in"]; root.InnerText = input; - - // If an attacker uses this for input: - // some textoscar - // Then the XML document will be: - // <allowedUser>oscar</allowedUser>some textalice } // } diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/ca3009.vb b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/ca3009.vb index ea0ff79c36f8d..fc92c93e7ca0f 100644 --- a/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/ca3009.vb +++ b/docs/fundamentals/code-analysis/quality-rules/snippets/vb/netfx/ca3009.vb @@ -17,11 +17,6 @@ Public Partial Class WebForm1 Dim input As String = Request.Form("in") root.InnerXml = input - - ' If an attacker uses this for input: - ' some textoscar - ' Then the XML document will be: - ' some textoscar End Sub ' End Class @@ -41,11 +36,6 @@ Public Partial Class WebForm2 Dim input As String = Request.Form("in") root.InnerText = input - - ' If an attacker uses this for input: - ' some textoscar - ' Then the XML document will be: - ' <allowedUser>oscar</allowedUser>some textalice End Sub ' End Class