Roslyn-analyzers: Unsecure Cyphers not detected by Microsoft Security Ruleset. NuGet package Microsoft.CodeAnalysis.FxCopAnalyzers 2.6.0

[lizetpena]

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers, version [2.6.0]
https://www.nuget.org/packages/Microsoft.CodeAnalysis.FxCopAnalyzers/

Analyzer

Example:

Repro steps

1. Add the NuGet package in question to a solution that you want to perform a static code analysis on.
2. Build solution
3. On the Code Analysis settings for the solution (Right click on the Solution Node on Solution Explorer). Configure the Microsoft Security Rules for all the projects in the solution.
4. Run Code Analysis on the Solution in question.

Expected behavior

CA5350: Do Not Use Weak Cryptographic Algorithms is expected on the output windows.

The static analysis should issue warnings on the use of unsecure cyphers such as DES, TripleDES for symmetric encryption and MD5 (hashing algorithm).
These cyphers are no longer deemed secure for systems that should comply with PCI-DSS and HIPAA.
CA5350: Do Not Use Weak Cryptographic Algorithms
does not show on the output windows and nothing with regards to the use of these classes is shown on the results of the static code analysis.

Actual behavior

The use of classes such as
System.Security.Cryptography.TripleDES
System.Security.Cryptography.DES
and
System.Security.Cryptography.MD5

won't show a warning on the output windows when the Microsoft Security Ruleset is used to do a static analysis on the source code present in the solution.

The projects on the solution are all compiled targeting .NET 4.5.x

[mavasani]

@jinujoseph We probably want to redirect this to the security team to triage this issue.

[jinujoseph]

@qinxgit , @tikiwanMS to follow-up here

[lizetpena]

@mavasani , @jinujoseph I'm part of a team at Microsoft as well. Those cyphers are, at present, considered weak by the several security compliance institutions. Would you like me to provide more information on this thread or offline?
Thank you! :)

[jinujoseph]

@lizetpena, as long as not confidential.. prefer sharing the information in the thread for others visibility.

[lizetpena]

@jinujoseph
Great, the information is not confidential.
This is why TripleDES is considered a weak symmetric block cipher/algorithm:
https://csrc.nist.gov/news/2017/update-to-current-use-and-deprecation-of-tdea

These are the attacks known and listed for TripleDES:
-Meet in the Middle Attacks
-Collision attacks

DES is an even weaker cipher.
https://csrc.nist.gov/csrc/media/publications/fips/46/3/archive/1999-10-25/documents/fips46-3.pdf

During a static code analysis using the Microsoft Recommended RuleSets the use of both classes
System.Security.Cryptography.TripleDES
System.Security.Cryptography.DES

should be noted in, at least, a warning.

Regarding MD5, there are stronger Hashing algorithms, as listed on
https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Secure-Hashing
MD5 is susceptible to collisions and has other known vulnerabilities.
https://csrc.nist.gov/News/2004/NIST-Brief-Comments-on-Recent-Cryptanalytic-Attack

Kind regards.
-Lizet

[lizetpena]

@mavasani, @jinujoseph, @qinxgit, @tikiwanMS,
Any updates?

Thank you.

[mavasani]

The fix here seems pretty simple - The security types validated by the analyzer needs to be updated. I will let @qinxgit @tikiwanMS confirm before working on this fix.

[qinxgit]

@mavasani I think something doesn't match here. Is the Fxcop analyzers now called .NET Framework Analyzers now? I can't find Fxcop analyzer folder in source now. If so, @lizetpena, can you try Microsoft.NetFramework.Analyzers package? These ciphers are already checked somewhere here https://github.com/dotnet/roslyn-analyzers/blob/b8fbcb0a85eecfe6b3b9e926e93e4a8a6bb1c669/src/Microsoft.NetFramework.Analyzers/Core/Helpers/SecurityTypes.cs#L136 .
The above "security types" is located in .NET Core analyzers but the actually the issue is talking about Fxcop Analyzers.

[333fred]

@qinxgit the original post in the issue is talking about the Microsoft.CodeAnalysis.FxCopAnalyzers nuget package, which is a metapackage that includes the .NET Core analyzers.

[qinxgit]

@333fred I see. But I failed to get a repro. The repro steps suggests a misunderstanding of using Roslyn Analyzers. Step 3 & 4 is not needed at all to use these rules. Steps 3 & 4 is the way to use the old Fxcop tool.

I successfully received CA5350 warning in my VS 2017 (15.5.6).

[mavasani]

@lizetpena - @qinxgit is correct. Sorry for my oversight on the repro steps. You just need to execute a build to execute the FXCop analyzers. Executing legacy FXCop ("Run Code Analysis") on a project which references FXCop analyzers nuget will only execute rules not ported to FXCop analyzers.

[lizetpena]

@mavasani, @qinxgit

You just need to execute a build to execute the FXCop analyzers.

I did that as well, without actually adding the nuget package in question:

Microsoft.CodeAnalysis.FxCopAnalyzers, version [2.6.0]
https://www.nuget.org/packages/Microsoft.CodeAnalysis.FxCopAnalyzers/

to the solution.

The Visual Studio Enterprise 2017 version 15.5.7, did not detect the use of the two classes (TripleDES and MD5), when running the Microsoft Security Rules, on the solution in question using the Analyze -> Run Code Analysis -> On Solution.
I configured the Code Analysis to only run the Microsoft Security Rules. I didn't want to run any other ruleset.
That ruleset only detected:
MSBUILD : warning CA2210: Microsoft.Design : Sign 'project.dll' with a strong name key.

What am I missing?

@qinxgit

@lizetpena, can you try Microsoft.NetFramework.Analyzers package?

The Microsoft.NetFramework.Analyzers package is a dependency of the Microsoft.CodeAnalysis.FxCopAnalyzers, and adding the Microsoft.CodeAnalysis.FxCopAnalyzers to the solution also adds the Microsoft.NetFramework.Analyzers nuget package.

[mavasani]

The Visual Studio Enterprise 2017 version 15.5.7, did not detect the use of the two classes (TripleDES and MD5), when running the Microsoft Security Rules, on the solution in question using the Analyze -> Run Code Analysis -> On Solution

@lizetpena seems there is still confusion here. "Run Code Analysis" invokes _legacy_ code analysis, which we are planning to deprecate soon and hence not aiming to fix. Can you please try the following repro:

1. Open your repro project.
2. Add a nuget package reference to https://www.nuget.org/packages/Microsoft.CodeAnalysis.FxCopAnalyzers/
3. Build project and you should see CA5350 warnings (you don't need to change any ruleset settings as this rule is enabled by default).

[lizetpena]

Hi @mavasani

"Run Code Analysis" invokes legacy code analysis

How old is legacy code analysis?
Let me rephrase the question, what target framework, should a project be set to compile with, so the Microsoft.CodeAnalysis.FxCopAnalyzers package can detect the use of two weak algorithms?

Could you kindly clarify that for me?

Also, which target frameworks won't be analyzed by the IDE Run Code Analysis?

I will, for sure, try the repro steps you mention as soon as I know the target framework that I must actually use in the project.

Thank you.

[mavasani]

@lizetpena Code analysis experience for managed projects is going to change very soon.

1. The "Run Code Analysis" menu will soon no longer be visible - today this executes a legacy tool FxCopCmd.exe which has been in support mode for ~5 years (only hangs/crashes are investigated).
2. Code analysis future is with analyzers integrated directly with the compiler/IDE. You add NuGet package reference to analyzers, which execute live in the IDE while typing and also execute automatically during build. There is no additional step to execute code analysis.
3. Analyzers might target different APIs, in which case they will executed on platforms where the API exists. Analyzers might also target general code quality, in which case they will execute on every platform. For your scenario, you just add a NuGet package reference to FXCopAnalyzers, and it will automatically execute the correct set of analyzers based on your platform.

[mavasani]

Tagging @gewarren @billwagner @kuhlenh - we need to create a single documentation/help link that we can point to customers to understand the current and future code analysis experience for managed projects.

[lizetpena]

@mavasani, @qinxgit
I created a repro solution to test if the ciphers were detected.
https://github.com/lizetpena/StaticCryptoAnalyzers

As suggested I installed the Microsoft.NetFramework.Analyzers 2.6.0 package, individually in two projects, one using C# 4.6 with .NET Core 2.0 as the target framework and one project targeting the full framework C# 4.5.2

The use of DES and TripleDES triggered a warning during the first build and rebuild in Visual Studio 2017, never on subsequent builds.
_However, the use of the hash MD5 never generated a warning._

The repro solution is public on:
https://github.com/lizetpena/StaticCryptoAnalyzers

Any plans on generating a warning for the use of weak hashing functions?

Thank you.

[lizetpena]

Should I open a new issue for the use of a weak hashing algorithm?

[qinxgit]

@lizetpena ,

I looked into your project. You are using the NetFramework.Analyzers. As we suggested, you should use Fxcop Analyzers instead of .NetFramework.Analyzers. The MD5 is not implemented in the NetFramework Analyzers by design.

[lizetpena]

Hey @qinxgit.
Thanks for the quick reply. So I shall use the FXCop analyzer 2.6.0 nuget package on both projects, and since this package has a dependency on the NetFramework.Analyzers, both nuget packages will be used on the static code analysis, thus, MD5 will be detected as a weak hashing algorithm?

Is that correct?

Kind regards,
-Lizet

PS. For cyber sec consultants that are relying on static code analysis to narrow down the amount of source code lines to review, this is very important.

[qinxgit]

Yes. That's correct. @lizetpena

[lizetpena]

Hi @qinxgit, @mavasani, @333fred
I did get all three warnings on the Build Output, when adding the FxCop 2.6.0 nuget package in question (Microsoft.CodeAnalysis.FxCopAnalyzers 2.6.0) individually to both projects in the solution. A Rebuild of the solution, triggers the warnings.

That will make my weekend 👍

The warnings were for the use of DES, TripleDES and MD5.

One project was using .NET Core 2.0 and C# 4.6 while the other one was using the full 4.5.2 framework with Common Language Runtime 4.0.

Does the Microsoft.CodeAnalysis.FxCopAnalyzers 2.6.0 nuget package go far back analyzing source code that target older common language runtimes? This is, older CLR versions, like 2.0 or 3.0?

If these ciphers were used on projects targeting compilation on CLR 2.0 (.NET 1.0 and 1.1) and CLR 3.0 (.NET 3.0 and 3.5), would the three ciphers be detected during builds with warnings?

There is no need to answer right away.
This might be a good thing to cover on documentation.

Thank you.

[mavasani]

Does the Microsoft.CodeAnalysis.FxCopAnalyzers 2.6.0 nuget package go far back analyzing source code that target older common language runtimes? This is, older CLR versions, like 2.0 or 3.0?

The analyzers are not platform or framework specific. They just check for presence of a type/API in your compilation (source and/or references), and if it exists, it will analyze and report diagnostics. If you see a different behavior, kindly let us know by filing separate issues. Thanks!

[mavasani]

Closing the issue as primary concern has been addressed.