Skip to content

Avoid redundant dataflow in security analyzers - #56415

Merged
Evangelink merged 3 commits into
dotnet:mainfrom
tannergooding:tannergooding-fix-crypto-analyzer-callsite-cost
Sep 30, 2026
Merged

Evangelink merged 3 commits into
dotnet:mainfrom
tannergooding:tannergooding-fix-crypto-analyzer-callsite-cost

Conversation

@tannergooding

Copy link
Copy Markdown
Member

Note

This PR description was drafted with AI assistance.

Fixes #56413.

The crypto rules were running whole-method dataflow for each literal byte-array source, even in generated methods that never call a relevant cryptographic API. With thousands of call sites, that repeats expensive work without a possible diagnostic. This change first checks whether a sink is reachable (including through helpers), skips sink-free methods, and runs taint analysis once per method instead of once per source. Value-content analysis is omitted when neither source recognition nor control-flow decisions need it; ambiguous cases still use the original full analysis. The reachability checks conservatively account for implicit calls, initializers, branches, and recursive helpers rather than assuming a simple source-operation walk covers the CFG.

The same approach avoids value-content flow in property-set rules unless a mapped constructor, property write, or relevant control-flow path requires it. Value-independent constructor overloads can be mapped directly, while value-dependent ones retain flow. Direct constant checks for AppContext.SetSwitch and Azure SharedAccessProtocol avoid building dataflow for already-known arguments.

Targeted local reproductions (not general speedup guarantees) measured the 2,048-call CA5390/CA5403 case at about 128s before vs. 2.3-2.5s after, a sink-free SQL CA3001 case at 1.209s vs. 0.014s, and a CA2329/CA2330 property-set case at 40.787s vs. 1.313s. The analyzer-free build of the crypto reproduction was 0.4-0.5s, so this reduces rather than eliminates analyzer overhead.

Skip whole-method flow when no relevant sink is reachable and omit value-content flow when it cannot affect mapped operations. Preserve diagnostic behavior for implicit calls, initializers, conditional branches, and helper methods.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 27, 2026 13:28
@tannergooding
tannergooding requested a review from a team as a code owner September 27, 2026 13:28
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 2 High severity

Open (2)
What changed in this PR

Optimizes security analyzers by avoiding unnecessary dataflow and adding broader C#/VB regression coverage.

Changes:

  • Adds sink-reachability and value-content prepasses.
  • Adds direct constant handling and constructor mappings.
  • Expands analyzer tests for helpers, initializers, branches, and VB.
File Description
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​UseSharedAccessProtocolHttpsOnlyTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​TaintedDataAnalyzerTestBase.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​ReviewCodeForXssVulnerabilitiesTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​ReviewCodeForSqlInjectionVulnerabilitiesTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​DoNotUseWeakKDFInsufficientIterationCountTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​DoNotUseInsecureDeserializerJsonNetWithoutBinderTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​DoNotInstallRootCertTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​DoNotHardCodeEncryptionKeyTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​DoNotHardCodeCertificateTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​tests/​Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/​Microsoft.NetCore.Analyzers/​Security/​DoNotDisableSchUseStrongCryptoTests.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Utilities/​FlowAnalysis/​FlowAnalysis/​Analysis/​TaintedDataAnalysis/​TaintedDataAnalysis.TaintedDataOperationVisitor.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Utilities/​FlowAnalysis/​FlowAnalysis/​Analysis/​TaintedDataAnalysis/​TaintedDataAnalysis.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Utilities/​FlowAnalysis/​FlowAnalysis/​Analysis/​PropertySetAnalysis/​PropertySetAnalysis.PropertySetDataFlowOperationVisitor.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Utilities/​FlowAnalysis/​FlowAnalysis/​Analysis/​PropertySetAnalysis/​PropertySetAnalysis.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Utilities/​FlowAnalysis/​FlowAnalysis/​Analysis/​PropertySetAnalysis/​ConstructorMapper.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Microsoft.CodeAnalysis.NetAnalyzers/​Microsoft.NetCore.Analyzers/​Security/​UseSharedAccessProtocolHttpsOnly.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Microsoft.CodeAnalysis.NetAnalyzers/​Microsoft.NetCore.Analyzers/​Security/​SourceTriggeredTaintedDataAnalyzerBase.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Microsoft.CodeAnalysis.NetAnalyzers/​Microsoft.NetCore.Analyzers/​Security/​DoNotUseWeakKDFInsufficientIterationCount.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Microsoft.CodeAnalysis.NetAnalyzers/​Microsoft.NetCore.Analyzers/​Security/​DoNotSetSwitch.cs Updated as part of this pull request.
src/​Microsoft.CodeAnalysis.NetAnalyzers/​src/​Microsoft.CodeAnalysis.NetAnalyzers/​Microsoft.NetCore.Analyzers/​Security/​DoNotInstallRootCert.cs Updated as part of this pull request.

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Drafted with AI assistance.

Found one performance regression-coverage gap.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Drafted with AI assistance.

Found three remaining performance regression-coverage gaps.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.

@tannergooding

Copy link
Copy Markdown
Member Author

@Evangelink anything else on this one?

@Evangelink
Evangelink merged commit 4d9bbbe into dotnet:main Sep 30, 2026
41 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 1, 2026
@jeffhandley

Copy link
Copy Markdown
Member

I will support backporting this into 11.0 servicing (post-GA)

@tannergooding

Copy link
Copy Markdown
Member Author

/backport to release/11.0.2xx

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0.2xx (link to workflow run)

@github-actions

Copy link
Copy Markdown
Contributor

@tannergooding backporting to release/11.0.2xx failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git cherry-pick 4d9bbbe02a27d3e522c8801694f0341e707d4711

Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotDisableSchUseStrongCryptoTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeCertificateTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeEncryptionKeyTests.cs
CONFLICT (content): Merge conflict in src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeEncryptionKeyTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotInstallRootCertTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotUseInsecureDeserializerJsonNetWithoutBinderTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotUseWeakKDFInsufficientIterationCountTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/ReviewCodeForSqlInjectionVulnerabilitiesTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/ReviewCodeForXssVulnerabilitiesTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/UseSharedAccessProtocolHttpsOnlyTests.cs
CONFLICT (content): Merge conflict in src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/UseSharedAccessProtocolHttpsOnlyTests.cs
error: could not apply 4d9bbbe02a... Avoid redundant dataflow in security analyzers (#56415)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Avoid redundant security analyzer dataflow
Using index info to reconstruct a base tree...
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotDisableSchUseStrongCryptoTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeCertificateTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeEncryptionKeyTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotInstallRootCertTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotUseInsecureDeserializerJsonNetWithoutBinderTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotUseWeakKDFInsufficientIterationCountTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/ReviewCodeForSqlInjectionVulnerabilitiesTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/ReviewCodeForXssVulnerabilitiesTests.cs
M	src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/UseSharedAccessProtocolHttpsOnlyTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotDisableSchUseStrongCryptoTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeCertificateTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeEncryptionKeyTests.cs
CONFLICT (content): Merge conflict in src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotHardCodeEncryptionKeyTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotInstallRootCertTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotUseInsecureDeserializerJsonNetWithoutBinderTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/DoNotUseWeakKDFInsufficientIterationCountTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/ReviewCodeForSqlInjectionVulnerabilitiesTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/ReviewCodeForXssVulnerabilitiesTests.cs
Auto-merging src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Security/UseSharedAccessProtocolHttpsOnlyTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Avoid redundant security analyzer dataflow
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

This branch had an error being deployed

1 failed deployment
copilot-pat-pool — 57a73a7c Deployed Sep 30, 2026 by Evangelink via pre_activation #7061
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CA5390 and CA5403 slow byte-array emission without cryptographic sinks

4 participants