From 7d493f2f18b62f43a8de643be6ce0adb55cca791 Mon Sep 17 00:00:00 2001 From: TillW <14997061+x789@users.noreply.github.com> Date: Sat, 8 Oct 2022 10:04:19 +0200 Subject: [PATCH 1/3] Remove CA2109 (roslyn-analyzers #5974) --- .../code-analysis/quality-rules/ca2109.md | 88 ------------------- .../code-analysis/quality-rules/index.md | 1 - .../quality-rules/security-warnings.md | 1 - .../snippets/csharp/all-rules/ca2109.cs | 26 ------ docs/fundamentals/toc.yml | 2 - 5 files changed, 118 deletions(-) delete mode 100644 docs/fundamentals/code-analysis/quality-rules/ca2109.md delete mode 100644 docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca2109.cs diff --git a/docs/fundamentals/code-analysis/quality-rules/ca2109.md b/docs/fundamentals/code-analysis/quality-rules/ca2109.md deleted file mode 100644 index cb94d249700d6..0000000000000 --- a/docs/fundamentals/code-analysis/quality-rules/ca2109.md +++ /dev/null @@ -1,88 +0,0 @@ ---- -title: "CA2109: Review visible event handlers (code analysis)" -description: "Learn about code analysis rule CA2109: Review visible event handlers" -ms.date: 11/04/2016 -ms.topic: reference -f1_keywords: -- CA2109 -- ReviewVisibleEventHandlers -helpviewer_keywords: -- ReviewVisibleEventHandlers -- CA2109 -author: gewarren -ms.author: gewarren ---- -# CA2109: Review visible event handlers - -| | Value | -|-|-| -| **Rule ID** |CA2109| -| **Category** |[Security](security-warnings.md)| -| **Fix is breaking or non-breaking** |Breaking| - -## Cause - -A public or protected event-handling method was detected. - -## Rule description - -An externally visible event-handling method presents a security issue that requires review. - -Do not expose event-handling methods unless absolutely necessary. An event handler, a delegate type, that invokes the exposed method can be added to any event as long as the handler and event signatures match. Events can potentially be raised by any code, and are frequently raised by highly trusted system code in response to user actions such as clicking a button. Adding a security check to an event-handling method does not prevent code from registering an event handler that invokes the method. - -A demand cannot reliably protect a method invoked by an event handler. Security demands help protect code from untrusted callers by examining the callers on the call stack. Code that adds an event handler to an event is not necessarily present on the call stack when the event handler's methods run. Therefore, the call stack might have only highly trusted callers when the event handler method is invoked. This causes demands made by the event handler method to succeed. Also, the demanded permission might be asserted when the method is invoked. For these reasons, the risk of not fixing a violation of this rule can only be assessed after reviewing the event-handling method. When you review your code, consider the following issues: - -- Does your event handler perform any operations that are dangerous or exploitable, such as asserting permissions or suppressing unmanaged code permission? - -- What are the security threats to and from your code because it can run at any time with only highly trusted callers on the stack? - -## How to fix violations - -To fix a violation of this rule, review the method and evaluate the following: - -- Can you make the event-handling method non-public? - -- Can you move all dangerous functionality out of the event handler? - -- If a security demand is imposed, can this be accomplished in some other manner? - -## When to suppress warnings - -Suppress a warning from this rule only after a careful security review to make sure that your code does not pose a security threat. - -## Suppress a warning - -If you just want to suppress a single violation, add preprocessor directives to your source file to disable and then re-enable the rule. - -```csharp -#pragma warning disable CA2109 -// The code that's violating the rule is on this line. -#pragma warning restore CA2109 -``` - -To disable the rule for a file, folder, or project, set its severity to `none` in the [configuration file](../configuration-files.md). - -```ini -[*.{cs,vb}] -dotnet_diagnostic.CA2109.severity = none -``` - -To disable this entire category of rules, set the severity for the category to `none` in the [configuration file](../configuration-files.md). - -```ini -[*.{cs,vb}] -dotnet_analyzer_diagnostic.category-Security.severity = none -``` - -For more information, see [How to suppress code analysis warnings](../suppress-warnings.md). - -## Example - -The following code shows an event-handling method that can be misused by malicious code. - -:::code language="csharp" source="snippets/csharp/all-rules/ca2109.cs" id="snippet1"::: - -## See also - -- -- diff --git a/docs/fundamentals/code-analysis/quality-rules/index.md b/docs/fundamentals/code-analysis/quality-rules/index.md index aac64835a6a3c..6d76fab3d71c3 100644 --- a/docs/fundamentals/code-analysis/quality-rules/index.md +++ b/docs/fundamentals/code-analysis/quality-rules/index.md @@ -161,7 +161,6 @@ The following table lists code quality analysis rules. > | [CA2018: The `count` argument to `Buffer.BlockCopy` should specify the number of bytes to copy](ca2018.md) | When using `Buffer.BlockCopy`, the `count` argument specifies the number of bytes to copy. You should only use `Array.Length` for the `count` argument on arrays whose elements are exactly one byte in size. `byte`, `sbyte`, and `bool` arrays have elements that are one byte in size. | > | [CA2100: Review SQL queries for security vulnerabilities](ca2100.md) | A method sets the System.Data.IDbCommand.CommandText property by using a string that is built from a string argument to the method. This rule assumes that the string argument contains user input. A SQL command string that is built from user input is vulnerable to SQL injection attacks. | > |[CA2101: Specify marshalling for P/Invoke string arguments](ca2101.md) | A platform invoke member allows partially trusted callers, has a string parameter, and does not explicitly marshal the string. This can cause a potential security vulnerability. | -> | [CA2109: Review visible event handlers](ca2109.md) | A public or protected event-handling method was detected. Event-handling methods should not be exposed unless absolutely necessary. | > | [CA2119: Seal methods that satisfy private interfaces](ca2119.md) | An inheritable public type provides an overridable method implementation of an internal (Friend in Visual Basic) interface. To fix a violation of this rule, prevent the method from being overridden outside the assembly. | > |[CA2153: Avoid handling Corrupted State Exceptions](ca2153.md) | Corrupted State Exceptions (CSEs) indicate that memory corruption exists in your process. Catching these rather than allowing the process to crash can lead to security vulnerabilities if an attacker can place an exploit into the corrupted memory region. | > | [CA2200: Rethrow to preserve stack details](ca2200.md) | An exception is rethrown and the exception is explicitly specified in the throw statement. If an exception is rethrown by specifying the exception in the throw statement, the list of method calls between the original method that threw the exception and the current method is lost. | diff --git a/docs/fundamentals/code-analysis/quality-rules/security-warnings.md b/docs/fundamentals/code-analysis/quality-rules/security-warnings.md index 953041b69b4a4..5c2cd692f45cd 100644 --- a/docs/fundamentals/code-analysis/quality-rules/security-warnings.md +++ b/docs/fundamentals/code-analysis/quality-rules/security-warnings.md @@ -22,7 +22,6 @@ Security rules support safer libraries and applications. These rules help preven |Rule|Description| |----------|-----------------| |[CA2100: Review SQL queries for security vulnerabilities](ca2100.md)|A method sets the System.Data.IDbCommand.CommandText property by using a string that is built from a string argument to the method. This rule assumes that the string argument contains user input. A SQL command string built from user input is vulnerable to SQL injection attacks.| -|[CA2109: Review visible event handlers](ca2109.md)|A public or protected event-handling method was detected. Event-handling methods should not be exposed unless absolutely necessary.| |[CA2119: Seal methods that satisfy private interfaces](ca2119.md)|An inheritable public type provides an overridable method implementation of an internal (Friend in Visual Basic) interface. To fix a violation of this rule, prevent the method from being overridden outside the assembly.| |[CA2153: Avoid Handling Corrupted State Exceptions](ca2153.md)|[Corrupted State Exceptions (CSE)](/archive/msdn-magazine/2009/february/clr-inside-out-handling-corrupted-state-exceptions) indicate that memory corruption exists in your process. Catching these rather than allowing the process to crash can lead to security vulnerabilities if an attacker can place an exploit into the corrupted memory region.| |[CA2300: Do not use insecure deserializer BinaryFormatter](ca2300.md)|Insecure deserializers are vulnerable when deserializing untrusted data. An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects.| diff --git a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca2109.cs b/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca2109.cs deleted file mode 100644 index 8d174b21a76f4..0000000000000 --- a/docs/fundamentals/code-analysis/quality-rules/snippets/csharp/all-rules/ca2109.cs +++ /dev/null @@ -1,26 +0,0 @@ -using System; -using System.Security.Permissions; - -namespace ca2109 -{ -#pragma warning disable SYSLIB0003 - // - public class HandleEvents - { - // Due to the access level and signature, a malicious caller could - // add this method to system-triggered events where all code in the call - // stack has the demanded permission. - - // Also, the demand might be canceled by an asserted permission. - - [SecurityPermissionAttribute(SecurityAction.Demand, UnmanagedCode = true)] - - // Violates rule: ReviewVisibleEventHandlers. - public static void SomeActionHappened(Object sender, EventArgs e) - { - Console.WriteLine("Do something dangerous from unmanaged code."); - } - } - // -#pragma warning restore SYSLIB0003 -} diff --git a/docs/fundamentals/toc.yml b/docs/fundamentals/toc.yml index a282fbdee6bc4..971a7caa9d3f1 100644 --- a/docs/fundamentals/toc.yml +++ b/docs/fundamentals/toc.yml @@ -1154,8 +1154,6 @@ items: href: code-analysis/quality-rules/security-warnings.md - name: CA2100 href: code-analysis/quality-rules/ca2100.md - - name: CA2109 - href: code-analysis/quality-rules/ca2109.md - name: CA2119 href: code-analysis/quality-rules/ca2119.md - name: CA2153 From 9948683679447b492371cb70f70f59b010ff18e5 Mon Sep 17 00:00:00 2001 From: TillW <14997061+x789@users.noreply.github.com> Date: Sun, 16 Oct 2022 12:15:37 +0200 Subject: [PATCH 2/3] Remove CA2109 from TOC and overviews --- docs/fundamentals/code-analysis/quality-rules/index.md | 1 - .../code-analysis/quality-rules/security-warnings.md | 1 - docs/fundamentals/toc.yml | 2 -- 3 files changed, 4 deletions(-) diff --git a/docs/fundamentals/code-analysis/quality-rules/index.md b/docs/fundamentals/code-analysis/quality-rules/index.md index aac64835a6a3c..6d76fab3d71c3 100644 --- a/docs/fundamentals/code-analysis/quality-rules/index.md +++ b/docs/fundamentals/code-analysis/quality-rules/index.md @@ -161,7 +161,6 @@ The following table lists code quality analysis rules. > | [CA2018: The `count` argument to `Buffer.BlockCopy` should specify the number of bytes to copy](ca2018.md) | When using `Buffer.BlockCopy`, the `count` argument specifies the number of bytes to copy. You should only use `Array.Length` for the `count` argument on arrays whose elements are exactly one byte in size. `byte`, `sbyte`, and `bool` arrays have elements that are one byte in size. | > | [CA2100: Review SQL queries for security vulnerabilities](ca2100.md) | A method sets the System.Data.IDbCommand.CommandText property by using a string that is built from a string argument to the method. This rule assumes that the string argument contains user input. A SQL command string that is built from user input is vulnerable to SQL injection attacks. | > |[CA2101: Specify marshalling for P/Invoke string arguments](ca2101.md) | A platform invoke member allows partially trusted callers, has a string parameter, and does not explicitly marshal the string. This can cause a potential security vulnerability. | -> | [CA2109: Review visible event handlers](ca2109.md) | A public or protected event-handling method was detected. Event-handling methods should not be exposed unless absolutely necessary. | > | [CA2119: Seal methods that satisfy private interfaces](ca2119.md) | An inheritable public type provides an overridable method implementation of an internal (Friend in Visual Basic) interface. To fix a violation of this rule, prevent the method from being overridden outside the assembly. | > |[CA2153: Avoid handling Corrupted State Exceptions](ca2153.md) | Corrupted State Exceptions (CSEs) indicate that memory corruption exists in your process. Catching these rather than allowing the process to crash can lead to security vulnerabilities if an attacker can place an exploit into the corrupted memory region. | > | [CA2200: Rethrow to preserve stack details](ca2200.md) | An exception is rethrown and the exception is explicitly specified in the throw statement. If an exception is rethrown by specifying the exception in the throw statement, the list of method calls between the original method that threw the exception and the current method is lost. | diff --git a/docs/fundamentals/code-analysis/quality-rules/security-warnings.md b/docs/fundamentals/code-analysis/quality-rules/security-warnings.md index 953041b69b4a4..5c2cd692f45cd 100644 --- a/docs/fundamentals/code-analysis/quality-rules/security-warnings.md +++ b/docs/fundamentals/code-analysis/quality-rules/security-warnings.md @@ -22,7 +22,6 @@ Security rules support safer libraries and applications. These rules help preven |Rule|Description| |----------|-----------------| |[CA2100: Review SQL queries for security vulnerabilities](ca2100.md)|A method sets the System.Data.IDbCommand.CommandText property by using a string that is built from a string argument to the method. This rule assumes that the string argument contains user input. A SQL command string built from user input is vulnerable to SQL injection attacks.| -|[CA2109: Review visible event handlers](ca2109.md)|A public or protected event-handling method was detected. Event-handling methods should not be exposed unless absolutely necessary.| |[CA2119: Seal methods that satisfy private interfaces](ca2119.md)|An inheritable public type provides an overridable method implementation of an internal (Friend in Visual Basic) interface. To fix a violation of this rule, prevent the method from being overridden outside the assembly.| |[CA2153: Avoid Handling Corrupted State Exceptions](ca2153.md)|[Corrupted State Exceptions (CSE)](/archive/msdn-magazine/2009/february/clr-inside-out-handling-corrupted-state-exceptions) indicate that memory corruption exists in your process. Catching these rather than allowing the process to crash can lead to security vulnerabilities if an attacker can place an exploit into the corrupted memory region.| |[CA2300: Do not use insecure deserializer BinaryFormatter](ca2300.md)|Insecure deserializers are vulnerable when deserializing untrusted data. An attacker could modify the serialized data to include unexpected types to inject objects with malicious side effects.| diff --git a/docs/fundamentals/toc.yml b/docs/fundamentals/toc.yml index a282fbdee6bc4..971a7caa9d3f1 100644 --- a/docs/fundamentals/toc.yml +++ b/docs/fundamentals/toc.yml @@ -1154,8 +1154,6 @@ items: href: code-analysis/quality-rules/security-warnings.md - name: CA2100 href: code-analysis/quality-rules/ca2100.md - - name: CA2109 - href: code-analysis/quality-rules/ca2109.md - name: CA2119 href: code-analysis/quality-rules/ca2119.md - name: CA2153 From a7b2841d74eaf500ae821bd1639b48c9c18f981b Mon Sep 17 00:00:00 2001 From: TillW <14997061+x789@users.noreply.github.com> Date: Sun, 16 Oct 2022 12:36:14 +0200 Subject: [PATCH 3/3] Add removal notice --- docs/fundamentals/code-analysis/quality-rules/ca2109.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/fundamentals/code-analysis/quality-rules/ca2109.md b/docs/fundamentals/code-analysis/quality-rules/ca2109.md index cb94d249700d6..50ab5e25d4262 100644 --- a/docs/fundamentals/code-analysis/quality-rules/ca2109.md +++ b/docs/fundamentals/code-analysis/quality-rules/ca2109.md @@ -24,6 +24,11 @@ ms.author: gewarren A public or protected event-handling method was detected. +> [!NOTE] +> This rule last shipped with Microsoft.CodeAnalysis.Analyzers v3.3.0. +> +> It was removed because the threat that the analyzer warned about (an untrusted intermediary hooking a privileged event handler to a privileged event invoker) did not exist since .NET 4.5. + ## Rule description An externally visible event-handling method presents a security issue that requires review.