Skip to content

Commit 54ea029

Browse files
mary-georgiou-sonarsourcesonartech
mary-georgiou-sonarsource
authored and
sonartech
committed
NET-1257 Fix S4790 FN: New HashData overloads not recognized
1 parent f47c985 commit 54ea029

File tree

8 files changed

+255
-82
lines changed

8 files changed

+255
-82
lines changed

‎analyzers/src/SonarAnalyzer.CSharp/Rules/Hotspots/CreatingHashAlgorithms.cs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,20 @@
1414
* along with this program; if not, see https://sonarsource.com/license/ssal/
1515
*/
1616

17-
namespace SonarAnalyzer.CSharp.Rules
17+
namespace SonarAnalyzer.CSharp.Rules;
18+
19+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
20+
public sealed class CreatingHashAlgorithms : CreatingHashAlgorithmsBase<SyntaxKind>
1821
{
19-
[DiagnosticAnalyzer(LanguageNames.CSharp)]
20-
public sealed class CreatingHashAlgorithms : CreatingHashAlgorithmsBase<SyntaxKind>
21-
{
22-
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
22+
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
23+
24+
public CreatingHashAlgorithms() : this(AnalyzerConfiguration.Hotspot) { }
2325

24-
public CreatingHashAlgorithms() : this(AnalyzerConfiguration.Hotspot) { }
26+
internal /*for testing*/ CreatingHashAlgorithms(IAnalyzerConfiguration configuration) : base(configuration) { }
2527

26-
internal /*for testing*/ CreatingHashAlgorithms(IAnalyzerConfiguration configuration) : base(configuration) { }
27-
}
28+
protected override bool IsUnsafeAlgorithm(SyntaxNode argumentNode, SemanticModel model) =>
29+
argumentNode as ArgumentSyntax is { } argument
30+
&& argument.Expression as MemberAccessExpressionSyntax is { } memberAccess
31+
&& memberAccess.Name.ToString() is "SHA1" or "MD5"
32+
&& model.GetSymbolInfo(memberAccess.Expression).Symbol.GetSymbolType().Is(KnownType.System_Security_Cryptography_HashAlgorithmName);
2833
}

‎analyzers/src/SonarAnalyzer.Core/Rules/Hotspots/CreatingHashAlgorithmsBase.cs

Lines changed: 90 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -16,75 +16,103 @@
1616

1717
using SonarAnalyzer.Core.Trackers;
1818

19-
namespace SonarAnalyzer.Core.Rules
19+
namespace SonarAnalyzer.Core.Rules;
20+
21+
public abstract class CreatingHashAlgorithmsBase<TSyntaxKind> : TrackerHotspotDiagnosticAnalyzer<TSyntaxKind>
22+
where TSyntaxKind : struct
2023
{
21-
public abstract class CreatingHashAlgorithmsBase<TSyntaxKind> : TrackerHotspotDiagnosticAnalyzer<TSyntaxKind>
22-
where TSyntaxKind : struct
23-
{
24-
protected const string DiagnosticId = "S4790";
25-
protected const string MessageFormat = "Make sure this weak hash algorithm is not used in a sensitive context here.";
24+
protected const string DiagnosticId = "S4790";
25+
protected const string MessageFormat = "Make sure this weak hash algorithm is not used in a sensitive context here.";
26+
27+
private const string CreateMethodName = "Create";
28+
private const string HashDataName = "HashData";
29+
private const string HashDataAsyncName = "HashDataAsync";
30+
private const string TryHashDataName = "TryHashData";
31+
32+
private readonly KnownType[] algorithmTypes =
33+
[
34+
KnownType.System_Security_Cryptography_DSA,
35+
KnownType.System_Security_Cryptography_HMACMD5,
36+
KnownType.System_Security_Cryptography_HMACRIPEMD160,
37+
KnownType.System_Security_Cryptography_HMACSHA1,
38+
KnownType.System_Security_Cryptography_MD5,
39+
KnownType.System_Security_Cryptography_RIPEMD160,
40+
KnownType.System_Security_Cryptography_SHA1
41+
];
2642

27-
private const string CreateMethodName = "Create";
43+
private readonly string[] unsafeAlgorithms =
44+
[
45+
"DSA",
46+
"System.Security.Cryptography.DSA",
47+
"HMACMD5",
48+
"System.Security.Cryptography.HMACMD5",
49+
"HMACRIPEMD160",
50+
"System.Security.Cryptography.HMACRIPEMD160",
51+
"HMACSHA1",
52+
"System.Security.Cryptography.HMACSHA1",
53+
"MD5",
54+
"System.Security.Cryptography.MD5",
55+
"RIPEMD160",
56+
"System.Security.Cryptography.RIPEMD160",
57+
"SHA1",
58+
"System.Security.Cryptography.SHA1",
59+
];
60+
61+
protected abstract bool IsUnsafeAlgorithm(SyntaxNode argumentNode, SemanticModel model);
62+
63+
protected CreatingHashAlgorithmsBase(IAnalyzerConfiguration configuration)
64+
: base(configuration, DiagnosticId, MessageFormat) { }
65+
66+
protected override void Initialize(TrackerInput input)
67+
{
68+
var oc = Language.Tracker.ObjectCreation;
69+
oc.Track(input, oc.WhenDerivesOrImplementsAny(algorithmTypes));
2870

29-
private readonly KnownType[] algorithmTypes =
30-
{
31-
KnownType.System_Security_Cryptography_DSA,
32-
KnownType.System_Security_Cryptography_HMACMD5,
33-
KnownType.System_Security_Cryptography_HMACRIPEMD160,
34-
KnownType.System_Security_Cryptography_HMACSHA1,
35-
KnownType.System_Security_Cryptography_MD5,
36-
KnownType.System_Security_Cryptography_RIPEMD160,
37-
KnownType.System_Security_Cryptography_SHA1
38-
};
71+
var tracker = Language.Tracker.Invocation;
72+
tracker.Track(input,
73+
tracker.MatchMethod(
74+
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, CreateMethodName),
75+
new MemberDescriptor(KnownType.System_Security_Cryptography_HMAC, CreateMethodName),
76+
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, CreateMethodName),
77+
new MemberDescriptor(KnownType.System_Security_Cryptography_RIPEMD160, CreateMethodName),
78+
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, CreateMethodName)),
79+
tracker.MethodHasParameters(0));
3980

40-
private readonly string[] unsafeAlgorithms =
41-
{
42-
"DSA",
43-
"System.Security.Cryptography.DSA",
44-
"HMACMD5",
45-
"System.Security.Cryptography.HMACMD5",
46-
"HMACRIPEMD160",
47-
"System.Security.Cryptography.HMACRIPEMD160",
48-
"HMACSHA1",
49-
"System.Security.Cryptography.HMACSHA1",
50-
"MD5",
51-
"System.Security.Cryptography.MD5",
52-
"RIPEMD160",
53-
"System.Security.Cryptography.RIPEMD160",
54-
"SHA1",
55-
"System.Security.Cryptography.SHA1"
56-
};
81+
tracker.Track(input,
82+
tracker.MatchMethod(
83+
new MemberDescriptor(KnownType.System_Security_Cryptography_AsymmetricAlgorithm, CreateMethodName),
84+
new MemberDescriptor(KnownType.System_Security_Cryptography_CryptoConfig, "CreateFromName"),
85+
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, CreateMethodName),
86+
new MemberDescriptor(KnownType.System_Security_Cryptography_HashAlgorithm, CreateMethodName),
87+
new MemberDescriptor(KnownType.System_Security_Cryptography_HMAC, CreateMethodName),
88+
new MemberDescriptor(KnownType.System_Security_Cryptography_KeyedHashAlgorithm, CreateMethodName),
89+
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, CreateMethodName),
90+
new MemberDescriptor(KnownType.System_Security_Cryptography_RIPEMD160, CreateMethodName),
91+
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, CreateMethodName)),
92+
tracker.ArgumentAtIndexIsAny(0, unsafeAlgorithms));
5793

58-
protected CreatingHashAlgorithmsBase(IAnalyzerConfiguration configuration)
59-
: base(configuration, DiagnosticId, MessageFormat) { }
94+
tracker.Track(input,
95+
tracker.MatchMethod(
96+
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, HashDataName),
97+
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, TryHashDataName),
98+
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, HashDataAsyncName),
99+
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, HashDataName),
100+
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, TryHashDataName),
101+
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, HashDataAsyncName)));
60102

61-
protected override void Initialize(TrackerInput input)
62-
{
63-
var oc = Language.Tracker.ObjectCreation;
64-
oc.Track(input, oc.WhenDerivesOrImplementsAny(algorithmTypes));
103+
tracker.Track(input,
104+
tracker.MatchMethod(
105+
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, HashDataName)),
106+
tracker.ArgumentAtIndexIs(3, IsUnsafeAlgorithm));
65107

66-
var t = Language.Tracker.Invocation;
67-
t.Track(input,
68-
t.MatchMethod(
69-
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, CreateMethodName),
70-
new MemberDescriptor(KnownType.System_Security_Cryptography_HMAC, CreateMethodName),
71-
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, CreateMethodName),
72-
new MemberDescriptor(KnownType.System_Security_Cryptography_RIPEMD160, CreateMethodName),
73-
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, CreateMethodName)),
74-
t.MethodHasParameters(0));
108+
tracker.Track(input,
109+
tracker.MatchMethod(
110+
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, HashDataName)),
111+
tracker.ArgumentAtIndexIs(1, IsUnsafeAlgorithm));
75112

76-
t.Track(input,
77-
t.MatchMethod(
78-
new MemberDescriptor(KnownType.System_Security_Cryptography_AsymmetricAlgorithm, CreateMethodName),
79-
new MemberDescriptor(KnownType.System_Security_Cryptography_CryptoConfig, "CreateFromName"),
80-
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, CreateMethodName),
81-
new MemberDescriptor(KnownType.System_Security_Cryptography_HashAlgorithm, CreateMethodName),
82-
new MemberDescriptor(KnownType.System_Security_Cryptography_HMAC, CreateMethodName),
83-
new MemberDescriptor(KnownType.System_Security_Cryptography_KeyedHashAlgorithm, CreateMethodName),
84-
new MemberDescriptor(KnownType.System_Security_Cryptography_MD5, CreateMethodName),
85-
new MemberDescriptor(KnownType.System_Security_Cryptography_RIPEMD160, CreateMethodName),
86-
new MemberDescriptor(KnownType.System_Security_Cryptography_SHA1, CreateMethodName)),
87-
t.ArgumentAtIndexIsAny(0, unsafeAlgorithms));
88-
}
113+
tracker.Track(input,
114+
tracker.MatchMethod(
115+
new MemberDescriptor(KnownType.System_Security_Cryptography_DSA, TryHashDataName)),
116+
tracker.ArgumentAtIndexIs(2, IsUnsafeAlgorithm));
89117
}
90118
}

‎analyzers/src/SonarAnalyzer.Core/Semantics/KnownType.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,7 @@ public sealed partial class KnownType
544544
public static readonly KnownType System_Security_Cryptography_ECDsa = new("System.Security.Cryptography.ECDsa");
545545
public static readonly KnownType System_Security_Cryptography_ECAlgorythm = new("System.Security.Cryptography.ECAlgorithm");
546546
public static readonly KnownType System_Security_Cryptography_HashAlgorithm = new("System.Security.Cryptography.HashAlgorithm");
547+
public static readonly KnownType System_Security_Cryptography_HashAlgorithmName = new("System.Security.Cryptography.HashAlgorithmName");
547548
public static readonly KnownType System_Security_Cryptography_HMAC = new("System.Security.Cryptography.HMAC");
548549
public static readonly KnownType System_Security_Cryptography_HMACMD5 = new("System.Security.Cryptography.HMACMD5");
549550
public static readonly KnownType System_Security_Cryptography_HMACRIPEMD160 = new("System.Security.Cryptography.HMACRIPEMD160");

‎analyzers/src/SonarAnalyzer.VisualBasic/Rules/Hotspots/CreatingHashAlgorithms.cs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,20 @@
1414
* along with this program; if not, see https://sonarsource.com/license/ssal/
1515
*/
1616

17-
namespace SonarAnalyzer.VisualBasic.Rules
17+
namespace SonarAnalyzer.VisualBasic.Rules;
18+
19+
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
20+
public sealed class CreatingHashAlgorithms : CreatingHashAlgorithmsBase<SyntaxKind>
1821
{
19-
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
20-
public sealed class CreatingHashAlgorithms : CreatingHashAlgorithmsBase<SyntaxKind>
21-
{
22-
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
22+
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
23+
24+
public CreatingHashAlgorithms() : this(AnalyzerConfiguration.Hotspot) { }
2325

24-
public CreatingHashAlgorithms() : this(AnalyzerConfiguration.Hotspot) { }
26+
internal /*for testing*/ CreatingHashAlgorithms(IAnalyzerConfiguration configuration) : base(configuration) { }
2527

26-
internal /*for testing*/ CreatingHashAlgorithms(IAnalyzerConfiguration configuration) : base(configuration) { }
27-
}
28+
protected override bool IsUnsafeAlgorithm(SyntaxNode argumentNode, SemanticModel model) =>
29+
argumentNode as SimpleArgumentSyntax is { } argument
30+
&& argument.Expression as MemberAccessExpressionSyntax is { } memberAccess
31+
&& memberAccess.Name.ToString() is "SHA1" or "MD5"
32+
&& model.GetSymbolInfo(memberAccess.Expression).Symbol.GetSymbolType().Is(KnownType.System_Security_Cryptography_HashAlgorithmName);
2833
}

‎analyzers/tests/SonarAnalyzer.Test/Rules/Hotspots/CreatingHashAlgorithmsTest.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ public void CreatingHashAlgorithms_VB_NetFx() =>
6060
public void CreatingHashAlgorithms_CS_Latest() =>
6161
builderCS.AddPaths("CreatingHashAlgorithms.Latest.cs").WithOptions(LanguageOptions.CSharpLatest).Verify();
6262

63+
[TestMethod]
64+
public void CreatingHashAlgorithms_VB_NET() =>
65+
builderVB.AddPaths("CreatingHashAlgorithms.NET.vb").Verify();
66+
6367
#endif
6468

6569
}

‎analyzers/tests/SonarAnalyzer.Test/TestCases/Hotspots/CreatingHashAlgorithms.Latest.cs

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Security.Cryptography;
44
using System.Text;
55
using System.Threading;
6+
using System.Threading.Tasks;
67

78
public class InsecureHashAlgorithm
89
{
@@ -30,12 +31,75 @@ void NewlinesInStringInterpolation()
3031
// https://github.com/SonarSource/sonar-dotnet/issues/8758
3132
public class Repro_FN_8758
3233
{
33-
void Method()
34+
public void Method()
3435
{
3536
var data = new byte[42];
37+
var hashBuffer = new byte[SHA1.HashSizeInBytes];
3638
using var stream = new System.IO.MemoryStream(data);
37-
SHA1.HashData(stream); // FN
38-
SHA1.HashData(data); // FN
39+
SHA1.HashData(stream); // Noncompliant
40+
SHA1.HashData(data); // Noncompliant
41+
if (SHA1.TryHashData(data, hashBuffer, out int bytesWrittenSHA1)) // Noncompliant
42+
{ }
43+
44+
MD5.HashData(data); // Noncompliant
45+
if (MD5.TryHashData(data, hashBuffer, out int bytesWrittenMD5)) // Noncompliant
46+
{ }
47+
}
48+
49+
public async Task Method2()
50+
{
51+
using var stream = new System.IO.MemoryStream(new byte[42]);
52+
await SHA1.HashDataAsync(stream); // Noncompliant
53+
await MD5.HashDataAsync(stream); // Noncompliant
54+
}
55+
56+
private sealed class MyDSA : DSA
57+
{
58+
public override byte[] CreateSignature(byte[] rgbHash) => [];
59+
60+
public override DSAParameters ExportParameters(bool includePrivateParameters) => new DSAParameters();
61+
62+
public override void ImportParameters(DSAParameters parameters)
63+
{ }
64+
65+
public override bool VerifySignature(byte[] rgbHash, byte[] rgbSignature) => true;
66+
67+
protected override byte[] HashData(byte[] data, int offset, int count, HashAlgorithmName hashAlgorithm) => [];
68+
69+
protected override byte[] HashData(Stream data, HashAlgorithmName hashAlgorithm) => [];
70+
71+
protected override bool TryHashData(ReadOnlySpan<byte> data, Span<byte> destination, HashAlgorithmName hashAlgorithm, out int bytesWritten)
72+
{
73+
bytesWritten = 0;
74+
return false;
75+
}
76+
77+
public void UseHashData()
78+
{
79+
var data = new byte[42];
80+
var hashBuffer = new byte[SHA1.HashSizeInBytes];
81+
using var stream = new System.IO.MemoryStream(data);
82+
83+
_ = HashData(data, 0, data.Length, HashAlgorithmName.SHA1); // Noncompliant
84+
_ = HashData(stream, HashAlgorithmName.SHA1); // Noncompliant
85+
if (TryHashData(data, hashBuffer, HashAlgorithmName.SHA1, out int bytesWrittenSHA1)) // Noncompliant
86+
{ }
87+
88+
_ = HashData(data, 0, data.Length, HashAlgorithmName.SHA3_256);
89+
_ = HashData(stream, HashAlgorithmName.SHA3_384);
90+
if (TryHashData(data, hashBuffer, HashAlgorithmName.SHA3_384, out int bytesWrittenSHA3))
91+
{ }
92+
93+
_ = base.HashData(data, 0, data.Length, HashAlgorithmName.SHA1); // Noncompliant
94+
_ = base.HashData(stream, HashAlgorithmName.SHA1); // Noncompliant
95+
if (base.TryHashData(data, hashBuffer, HashAlgorithmName.SHA1, out int bytesWrittenSHA1Base)) // Noncompliant
96+
{ }
97+
98+
_ = base.HashData(data, 0, data.Length, HashAlgorithmName.SHA3_256);
99+
_ = base.HashData(stream, HashAlgorithmName.SHA3_384);
100+
if (base.TryHashData(data, hashBuffer, HashAlgorithmName.SHA3_384, out int bytesWrittenSHA3Base))
101+
{ }
102+
}
39103
}
40104
}
41105

‎analyzers/tests/SonarAnalyzer.Test/TestCases/Hotspots/CreatingHashAlgorithms.NET.vb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
Imports System
2+
Imports System.IO
3+
Imports System.Security.Cryptography
4+
Imports System.Threading.Tasks
5+
6+
Namespace Tests.Diagnostics
7+
8+
Public Class Repro_FN_8758
9+
Public Sub Method()
10+
Dim data As Byte() = New Byte(41) {}
11+
Dim hashBuffer As Byte() = New Byte(SHA1.HashSizeInBytes - 1) {}
12+
Using stream As New MemoryStream(data)
13+
SHA1.HashData(stream) ' Noncompliant
14+
SHA1.HashData(data) ' Noncompliant
15+
Dim bytesWrittenSHA1 As Integer
16+
If SHA1.TryHashData(data, hashBuffer, bytesWrittenSHA1) Then ' Noncompliant
17+
End If
18+
MD5.HashData(data) ' Noncompliant
19+
Dim bytesWrittenMD5 As Integer
20+
If MD5.TryHashData(data, hashBuffer, bytesWrittenMD5) Then ' Noncompliant
21+
End If
22+
End Using
23+
End Sub
24+
25+
Public Async Function Method2() As Task
26+
Using stream As New MemoryStream(New Byte(41) {})
27+
Await SHA1.HashDataAsync(stream) ' Noncompliant
28+
Await MD5.HashDataAsync(stream) ' Noncompliant
29+
End Using
30+
End Function
31+
32+
End Class
33+
34+
35+
End Namespace

0 commit comments

Comments
 (0)