Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"CA5390: Do Not Hard Code Encryption Key" does not work with initialized fields #2905

Open
avivanoff opened this issue Oct 3, 2019 · 5 comments
Assignees
Labels
Area-Microsoft.CodeAnalysis.NetAnalyzers Bug The product is not behaving according to its current intended design Category-Security
Milestone

Comments

@avivanoff
Copy link

avivanoff commented Oct 3, 2019

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.5 (Latest)

Diagnostic ID

CA5390

Repro steps

  1. Create a simple console app.
  2. Add FxCop analyzers and make sure CA5390 is enabled.
  3. Add the following code:
using System;
using System.Security.Cryptography;

internal static class Program
{
    private static readonly Byte[] _key = { 1, 2, 3 };

    private static void Main()
    {
        using (var aes = Aes.Create())
        {
            aes.Key = Program._key;
        }
    }
}

Expected behavior

CA5390 warning/error should be triggered.

Actual behavior

CA5390 warning/error is not triggered.

@mavasani mavasani added Area-Microsoft.CodeAnalysis.NetAnalyzers Bug The product is not behaving according to its current intended design Category-Security labels Oct 3, 2019
@mavasani mavasani added this to the vNext milestone Oct 3, 2019
@dotpaul
Copy link
Contributor

dotpaul commented Oct 3, 2019

Hi @avivanoff, the example code should trigger CA5390, and it seems to trigger for me.
image

@avivanoff
Copy link
Author

Strange. When I did a full rebuild, it started to work.

@mavasani
Copy link
Member

mavasani commented Oct 3, 2019

@avivanoff I think that is likely because lot of security analyzers are build-only by default analyzers (compilation end based). They only run in IDE live analysis when you enable full solution analysis.

dotpaul added a commit to dotpaul/roslyn-analyzers that referenced this issue Oct 4, 2019
@avivanoff
Copy link
Author

Actually, I believe the rule does have a problem. The following code is not marked:

internal static class Program
{
    private static readonly Byte[] _key = { 1, 2, 3 };

    private static void Main()
    {
        using (var aes = Aes.Create())
        {
            aes.Key = Program._key;
        }
    }
}

@avivanoff avivanoff reopened this Oct 4, 2019
@dotpaul
Copy link
Contributor

dotpaul commented Oct 4, 2019

Thanks for reporting, @avivanoff ! We currently don't analyze field initializers, which is why that example doesn't produce a warning. For this rule, I think we'll have to investigate how we can take field initializers into account, since I'm guessing initializing byte arrays is common enough to be worth analyzing.

@dotpaul dotpaul changed the title "CA5390: Do Not Hard Code Encryption Key" does not work "CA5390: Do Not Hard Code Encryption Key" does not work with initialized fields Oct 4, 2019
dotpaul added a commit that referenced this issue Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Microsoft.CodeAnalysis.NetAnalyzers Bug The product is not behaving according to its current intended design Category-Security
Projects
None yet
Development

No branches or pull requests

4 participants