From 13bf3cdc6fc14d53083230ad5d2047f453a1c1af Mon Sep 17 00:00:00 2001 From: LingxiaChen Date: Mon, 29 Jul 2019 11:44:55 +0800 Subject: [PATCH 1/2] Hopefully fixed the bug. --- .../SourceTriggeredTaintedDataAnalyzerBase.cs | 2 +- .../DoNotHardCodeEncryptionKeyTests.cs | 52 +++++++++++++------ ...ataAnalysis.TaintedDataOperationVisitor.cs | 6 ++- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs b/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs index 8558f6733e..c13f6022a6 100644 --- a/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs +++ b/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs @@ -179,7 +179,7 @@ public override void Initialize(AnalysisContext context) operationAnalysisContext => { IArrayInitializerOperation arrayInitializerOperation = (IArrayInitializerOperation)operationAnalysisContext.Operation; - if (sourceInfoSymbolMap.IsSourceConstantArrayOfType(arrayInitializerOperation.Parent.Type as IArrayTypeSymbol)) + if (sourceInfoSymbolMap.IsSourceConstantArrayOfType(arrayInitializerOperation.GetAncestor(OperationKind.ArrayCreation).Type as IArrayTypeSymbol)) { lock (rootOperationsNeedingAnalysis) { diff --git a/src/Microsoft.NetCore.Analyzers/UnitTests/Security/DoNotHardCodeEncryptionKeyTests.cs b/src/Microsoft.NetCore.Analyzers/UnitTests/Security/DoNotHardCodeEncryptionKeyTests.cs index d3dcce2f72..ae3bba6ad8 100644 --- a/src/Microsoft.NetCore.Analyzers/UnitTests/Security/DoNotHardCodeEncryptionKeyTests.cs +++ b/src/Microsoft.NetCore.Analyzers/UnitTests/Security/DoNotHardCodeEncryptionKeyTests.cs @@ -79,7 +79,7 @@ public void TestMethod(byte[] someOtherBytesForIV) } [Fact] - public void Test_HardcodedInbyteArray_CreateEncryptor_Diagnostic() + public void Test_HardcodedInByteArray_CreateEncryptor_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -94,11 +94,11 @@ public void TestMethod(byte[] someOtherBytesForIV) rijn.CreateEncryptor(rgbKey, someOtherBytesForIV); } }", - GetCSharpResultAt(11, 9, 9, 36, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(11, 9, 9, 25, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] - public void Test_HardcodedInbyteArray_CreateDecryptor_Diagnostic() + public void Test_HardcodedInByteArray_CreateDecryptor_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -113,11 +113,11 @@ public void TestMethod(byte[] someOtherBytesForIV) rijn.CreateDecryptor(rgbKey, someOtherBytesForIV); } }", - GetCSharpResultAt(11, 9, 9, 36, "ICryptoTransform SymmetricAlgorithm.CreateDecryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(11, 9, 9, 25, "ICryptoTransform SymmetricAlgorithm.CreateDecryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] - public void Test_HardcodedInbyteArrayWithVariable_CreateEncryptor_Diagnostic() + public void Test_HardcodedInByteArrayWithVariable_CreateEncryptor_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -133,11 +133,11 @@ public void TestMethod(byte[] someOtherBytesForIV) rijn.CreateEncryptor(rgbKey, someOtherBytesForIV); } }", - GetCSharpResultAt(12, 9, 10, 36, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(12, 9, 10, 25, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] - public void Test_HardcodedInbyteArray_KeyProperty_Diagnostic() + public void Test_HardcodedInByteArray_KeyProperty_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -152,11 +152,11 @@ public void TestMethod(byte[] someOtherBytesForIV) rijn.Key = rgbKey; } }", - GetCSharpResultAt(11, 9, 9, 36, "byte[] SymmetricAlgorithm.Key", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(11, 9, 9, 25, "byte[] SymmetricAlgorithm.Key", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] - public void Test_HardcodedInbyteArray_CreateEncryptorFromDerivedClassOfSymmetricAlgorithm_Diagnostic() + public void Test_HardcodedInByteArray_CreateEncryptorFromDerivedClassOfSymmetricAlgorithm_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -171,11 +171,11 @@ public void TestMethod(byte[] someOtherBytesForIV) aes.CreateEncryptor(rgbKey, someOtherBytesForIV); } }", - GetCSharpResultAt(11, 9, 9, 36, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(11, 9, 9, 25, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] - public void Test_HardcodedInbyteArray_CreateEncryptor_Multivalues_Diagnostic() + public void Test_HardcodedInByteArray_CreateEncryptor_Multivalues_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -197,12 +197,12 @@ public void TestMethod(byte[] someOtherBytesForIV) rijn.CreateEncryptor(rgbKey, someOtherBytesForIV); } }", - GetCSharpResultAt(18, 9, 14, 33, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)"), - GetCSharpResultAt(18, 9, 9, 36, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(18, 9, 14, 22, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)"), + GetCSharpResultAt(18, 9, 9, 25, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] - public void Test_HardcodedInbyteArray_CreateEncryptor_WithoutAssignment_Diagnostic() + public void Test_HardcodedInByteArray_CreateEncryptor_WithoutAssignment_Diagnostic() { VerifyCSharpWithDependencies(@" using System; @@ -216,7 +216,7 @@ public void TestMethod(byte[] someOtherBytesForIV) rijn.CreateEncryptor(new byte[] {1, 2, 3}, someOtherBytesForIV); } }", - GetCSharpResultAt(10, 9, 10, 41, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + GetCSharpResultAt(10, 9, 10, 30, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); } [Fact] @@ -241,7 +241,7 @@ public void TestMethod(byte[] someOtherBytesForIV, byte[] rgbKey) rijn.CreateEncryptor(rgbKey, someOtherBytesForIV); } }", - GetCSharpResultAt(17, 9, 13, 33, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV, byte[] rgbKey)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV, byte[] rgbKey)")); + GetCSharpResultAt(17, 9, 13, 22, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV, byte[] rgbKey)", "byte[]", "void TestClass.TestMethod(byte[] someOtherBytesForIV, byte[] rgbKey)")); } [Fact] @@ -292,6 +292,26 @@ public void CreateEncryptor(byte[] rgbKey) GetCSharpResultAt(16, 9, 9, 22, "byte[] SymmetricAlgorithm.Key", "void TestClass.CreateEncryptor(byte[] rgbKey)", "byte[] Convert.FromBase64String(string s)", "void TestClass.TestMethod()")); } + [Fact] + public void Test_HardcodedIn2DByteArray_CreateEncryptor_Diagnostic() + { + VerifyCSharpWithDependencies(@" +using System; +using System.Linq; +using System.Security.Cryptography; + +class TestClass +{ + public void TestMethod(byte[] someOtherBytesForIV) + { + byte[,] rgbKey = new byte[,] { { 1, 2, 3 }, { 4, 5, 6 } }; + SymmetricAlgorithm rijn = SymmetricAlgorithm.Create(); + rijn.CreateEncryptor(rgbKey.Cast().ToArray(), someOtherBytesForIV); + } +}", + GetCSharpResultAt(12, 9, 10, 26, "ICryptoTransform SymmetricAlgorithm.CreateEncryptor(byte[] rgbKey, byte[] rgbIV)", "void TestClass.TestMethod(byte[] someOtherBytesForIV)", "byte[,]", "void TestClass.TestMethod(byte[] someOtherBytesForIV)")); + } + [Fact] public void Test_NotHardcoded_CreateEncryptor_NoDiagnostic() { diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs index 4158b516b3..96896ad190 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs @@ -343,10 +343,12 @@ public override TaintedDataAbstractValue VisitArrayInitializer(IArrayInitializer result = TaintedDataAbstractValue.MergeTainted(taintedAbstractValues); } - if (this.DataFlowAnalysisContext.SourceInfos.IsSourceConstantArrayOfType(operation.Parent.Type as IArrayTypeSymbol) + IArrayCreationOperation arrayCreationOperation = operation.GetAncestor(OperationKind.ArrayCreation); + IArrayTypeSymbol arrayTypeSymbol = arrayCreationOperation.Type as IArrayTypeSymbol; + if (this.DataFlowAnalysisContext.SourceInfos.IsSourceConstantArrayOfType(arrayTypeSymbol) && operation.ElementValues.All(s => GetValueContentAbstractValue(s).IsLiteralState)) { - TaintedDataAbstractValue taintedDataAbstractValue = TaintedDataAbstractValue.CreateTainted(operation.Parent.Type, operation.Syntax, this.OwningSymbol); + TaintedDataAbstractValue taintedDataAbstractValue = TaintedDataAbstractValue.CreateTainted(arrayTypeSymbol, arrayCreationOperation.Syntax, this.OwningSymbol); result = result == null ? taintedDataAbstractValue : TaintedDataAbstractValue.MergeTainted(result, taintedDataAbstractValue); } From 9d097367fcf6056302a7b8553cf5390e542b0d36 Mon Sep 17 00:00:00 2001 From: LingxiaChen Date: Mon, 29 Jul 2019 12:57:59 +0800 Subject: [PATCH 2/2] Update. --- .../Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs | 3 ++- .../TaintedDataAnalysis.TaintedDataOperationVisitor.cs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs b/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs index c13f6022a6..443c9df28d 100644 --- a/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs +++ b/src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs @@ -179,7 +179,8 @@ public override void Initialize(AnalysisContext context) operationAnalysisContext => { IArrayInitializerOperation arrayInitializerOperation = (IArrayInitializerOperation)operationAnalysisContext.Operation; - if (sourceInfoSymbolMap.IsSourceConstantArrayOfType(arrayInitializerOperation.GetAncestor(OperationKind.ArrayCreation).Type as IArrayTypeSymbol)) + if (arrayInitializerOperation.GetAncestor(OperationKind.ArrayCreation)?.Type is IArrayTypeSymbol arrayTypeSymbol + && sourceInfoSymbolMap.IsSourceConstantArrayOfType(arrayTypeSymbol)) { lock (rootOperationsNeedingAnalysis) { diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs index 96896ad190..0ae7919c5e 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs @@ -344,8 +344,8 @@ public override TaintedDataAbstractValue VisitArrayInitializer(IArrayInitializer } IArrayCreationOperation arrayCreationOperation = operation.GetAncestor(OperationKind.ArrayCreation); - IArrayTypeSymbol arrayTypeSymbol = arrayCreationOperation.Type as IArrayTypeSymbol; - if (this.DataFlowAnalysisContext.SourceInfos.IsSourceConstantArrayOfType(arrayTypeSymbol) + if (arrayCreationOperation?.Type is IArrayTypeSymbol arrayTypeSymbol + && this.DataFlowAnalysisContext.SourceInfos.IsSourceConstantArrayOfType(arrayTypeSymbol) && operation.ElementValues.All(s => GetValueContentAbstractValue(s).IsLiteralState)) { TaintedDataAbstractValue taintedDataAbstractValue = TaintedDataAbstractValue.CreateTainted(arrayTypeSymbol, arrayCreationOperation.Syntax, this.OwningSymbol);