Skip to content

Commit

Permalink
Merge pull request #3014 from LLLXXXCCC/DoNotHardCodeCertificate
Browse files Browse the repository at this point in the history 
Do not hard code certificate
  • Loading branch information
@LLLXXXCCC
LLLXXXCCC committed Nov 13, 2019
2 parents 6e190ed + 6a5b095 commit dbc50b1
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 69 deletions.
96 changes: 40 additions & 56 deletions src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis;
Expand Down Expand Up @@ -50,7 +51,8 @@ public override void Initialize(AnalysisContext context)
context.RegisterCompilationStartAction(
(CompilationStartAnalysisContext compilationContext) =>
{
TaintedDataConfig taintedDataConfig = TaintedDataConfig.GetOrCreate(compilationContext.Compilation);
Compilation compilation = compilationContext.Compilation;
TaintedDataConfig taintedDataConfig = TaintedDataConfig.GetOrCreate(compilation);
TaintedDataSymbolMap<SourceInfo> sourceInfoSymbolMap = taintedDataConfig.GetSourceSymbolMap(this.SinkKind);
if (sourceInfoSymbolMap.IsEmpty)
{
Expand All @@ -67,16 +69,45 @@ public override void Initialize(AnalysisContext context)
operationBlockStartContext =>
{
ISymbol owningSymbol = operationBlockStartContext.OwningSymbol;
if (owningSymbol.IsConfiguredToSkipAnalysis(operationBlockStartContext.Options,
TaintedDataEnteringSinkDescriptor, operationBlockStartContext.Compilation, operationBlockStartContext.CancellationToken))
AnalyzerOptions options = operationBlockStartContext.Options;
CancellationToken cancellationToken = operationBlockStartContext.CancellationToken;
if (owningSymbol.IsConfiguredToSkipAnalysis(options, TaintedDataEnteringSinkDescriptor, compilation, cancellationToken))
{
return;
}
PointsToAnalysisResult pointsToResult = null;
bool pointsToComputed = false;
ValueContentAnalysisResult valueContentResult = null;
bool valueContentComputed = false;
ControlFlowGraph cfg = operationBlockStartContext.OperationBlocks.GetControlFlowGraph();
WellKnownTypeProvider wellKnownTypeProvider = WellKnownTypeProvider.GetOrCreate(compilation);
InterproceduralAnalysisConfiguration interproceduralAnalysisConfiguration = InterproceduralAnalysisConfiguration.Create(
options,
SupportedDiagnostics,
defaultInterproceduralAnalysisKind: InterproceduralAnalysisKind.ContextSensitive,
cancellationToken: cancellationToken);
Lazy<PointsToAnalysisResult> pointsToFactory = new Lazy<PointsToAnalysisResult>(
() =>
{
return PointsToAnalysis.TryGetOrComputeResult(
cfg,
owningSymbol,
options,
wellKnownTypeProvider,
interproceduralAnalysisConfiguration,
interproceduralAnalysisPredicateOpt: null);
});
Lazy<(PointsToAnalysisResult, ValueContentAnalysisResult)> valueContentFactory = new Lazy<(PointsToAnalysisResult, ValueContentAnalysisResult)>(
() =>
{
ValueContentAnalysisResult valuecontentAnalysisResult = ValueContentAnalysis.TryGetOrComputeResult(
cfg,
owningSymbol,
options,
wellKnownTypeProvider,
interproceduralAnalysisConfiguration,
out _,
out PointsToAnalysisResult p);
return (p, valuecontentAnalysisResult);
});
PooledHashSet<IOperation> rootOperationsNeedingAnalysis = PooledHashSet<IOperation>.GetInstance();
Expand Down Expand Up @@ -105,55 +136,8 @@ public override void Initialize(AnalysisContext context)
if (sourceInfoSymbolMap.IsSourceMethod(
invocationOperation.TargetMethod,
invocationOperation.Arguments,
new Lazy<PointsToAnalysisResult>(
() =>
{
if (!pointsToComputed)
{
pointsToResult = PointsToAnalysis.TryGetOrComputeResult(
cfg,
owningSymbol,
operationAnalysisContext.Options,
WellKnownTypeProvider.GetOrCreate(operationAnalysisContext.Compilation),
InterproceduralAnalysisConfiguration.Create(
operationAnalysisContext.Options,
SupportedDiagnostics,
defaultInterproceduralAnalysisKind: InterproceduralAnalysisKind.ContextSensitive,
cancellationToken: operationAnalysisContext.CancellationToken),
interproceduralAnalysisPredicateOpt: null);
pointsToComputed = true;
}
return pointsToResult;
}),
new Lazy<ValueContentAnalysisResult>(
() =>
{
if (!valueContentComputed)
{
valueContentResult = ValueContentAnalysis.TryGetOrComputeResult(
cfg,
owningSymbol,
operationAnalysisContext.Options,
WellKnownTypeProvider.GetOrCreate(operationAnalysisContext.Compilation),
InterproceduralAnalysisConfiguration.Create(
operationAnalysisContext.Options,
SupportedDiagnostics,
defaultInterproceduralAnalysisKind: InterproceduralAnalysisKind.ContextSensitive,
cancellationToken: operationAnalysisContext.CancellationToken),
out _,
out PointsToAnalysisResult p);
if (p != null && pointsToResult == null)
{
pointsToResult = p;
pointsToComputed = true;
}
valueContentComputed = true;
}
return valueContentResult;
}),
pointsToFactory,
valueContentFactory,
out _))
{
lock (rootOperationsNeedingAnalysis)
Expand Down
51 changes: 51 additions & 0 deletions src/Microsoft.NetCore.Analyzers/UnitTests/Security/DoNotHardCodeCertificateTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Test.Utilities;
using Xunit;
using Xunit.Abstractions;

Expand Down Expand Up @@ -383,6 +384,56 @@ public void TestMethod(byte[] bytes, string path)
}");
}

// Didn't find out what causes NRE.
[Fact, WorkItem(3012, "https://github.com/dotnet/roslyn-analyzers/issues/3012")]
public void Test_ExampleCodeFromTheIssue_NoDiagnostic()
{
VerifyCSharp(@"
using System;
using System.Globalization;
using System.IO;
using System.Security;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using System.Text.RegularExpressions;
class Constants
{
public static Regex UnhashedNameIdRegex = new Regex(@""^[a-zA-Z0-9]\d{2}[a-zA-Z0-9](-\d{3}){2}[A-Za-z0-9]$"");
}
class TestClass
{
public static string Calculate(string unhashedNameId)
{
if (string.IsNullOrWhiteSpace(unhashedNameId))
{
throw new ArgumentNullException(nameof(unhashedNameId), $""{ nameof(unhashedNameId)} must not be null, empty or whitespace."");
}
if (!Constants.UnhashedNameIdRegex.IsMatch(unhashedNameId))
{
throw new ArgumentException($""{ nameof(unhashedNameId)} does not match '{Constants.UnhashedNameIdRegex}'."", nameof(unhashedNameId));
}
using (var sha = new SHA256Managed())
{
byte[] textData = Encoding.UTF8.GetBytes(unhashedNameId);
byte[] crypto = sha.ComputeHash(textData);
var nameId = new StringBuilder();
foreach (byte hash in crypto)
{
nameId.Append(hash.ToString(""x2"", CultureInfo.InvariantCulture));
}
return nameId.ToString();
}
}
}");
}

protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
{
return new DoNotHardCodeCertificate();
Expand Down
2 changes: 1 addition & 1 deletion ...wAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.TaintedDataOperationVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public override TaintedDataAbstractValue VisitObjectCreation(IObjectCreationOper
method,
visitedArguments,
new Lazy<PointsToAnalysisResult>(() => DataFlowAnalysisContext.PointsToAnalysisResultOpt),
new Lazy<ValueContentAnalysisResult>(() => DataFlowAnalysisContext.ValueContentAnalysisResultOpt),
new Lazy<(PointsToAnalysisResult, ValueContentAnalysisResult)>(() => (DataFlowAnalysisContext.PointsToAnalysisResultOpt, DataFlowAnalysisContext.ValueContentAnalysisResultOpt)),
out taintedTargets))
{
foreach (string taintedTarget in taintedTargets)
Expand Down
38 changes: 26 additions & 12 deletions .../FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataSymbolMapExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,21 @@ internal static class TaintedDataSymbolMapExtensions
/// <param name="sourceSymbolMap"></param>
/// <param name="method"></param>
/// <param name="arguments"></param>
/// <param name="pointsToAnalysisResult">If the method needs to do PointsToAnalysis, the PointsToAnalysis result will be produced by the passed value factory.</param>
/// <param name="valueContentAnalysisResult">If the method needs to do ValueContentAnalysis, the ValueContentAnalysis result will be produced by the passed value factory.</param>
/// <param name="pointsToFactory">If the method needs to do PointsToAnalysis, the PointsToAnalysis result will be produced by the passed value factory.</param>
/// <param name="valueContentFactory">If the method needs to do ValueContentAnalysis, the ValueContentAnalysis result will be produced by the passed value factory.</param>
/// <param name="allTaintedTargets"></param>
/// <returns></returns>
public static bool IsSourceMethod(
this TaintedDataSymbolMap<SourceInfo> sourceSymbolMap,
IMethodSymbol method,
ImmutableArray<IArgumentOperation> arguments,
Lazy<PointsToAnalysisResult> pointsToAnalysisResult,
Lazy<ValueContentAnalysisResult> valueContentAnalysisResult,
Lazy<PointsToAnalysisResult> pointsToFactory,
Lazy<(PointsToAnalysisResult p, ValueContentAnalysisResult v)> valueContentFactory,
out PooledHashSet<string> allTaintedTargets)
{
allTaintedTargets = null;
PointsToAnalysisResult pointsToAnalysisResult = null;
ValueContentAnalysisResult valueContentAnalysisResult = null;
foreach (SourceInfo sourceInfo in sourceSymbolMap.GetInfosForType(method.ContainingType))
{
foreach ((MethodMatcher methodMatcher, ImmutableHashSet<string> taintedTargets) in sourceInfo.TaintedMethods)
Expand All @@ -53,13 +55,19 @@ internal static class TaintedDataSymbolMapExtensions

foreach ((MethodMatcher methodMatcher, ImmutableHashSet<(PointsToCheck pointsToCheck, string)> pointsToTaintedTargets) in sourceInfo.TaintedMethodsNeedsPointsToAnalysis)
{
if (methodMatcher(method.Name, arguments))
if (pointsToTaintedTargets.Any() && methodMatcher(method.Name, arguments))
{
pointsToAnalysisResult ??= pointsToFactory.Value;
if (pointsToAnalysisResult == null)
{
break;
}

IEnumerable<(PointsToCheck, string target)> positivePointsToTaintedTargets = pointsToTaintedTargets.Where(s =>
s.pointsToCheck(
arguments.Select(o =>
pointsToAnalysisResult.Value[o.Kind, o.Syntax]).ToImmutableArray()));
if (positivePointsToTaintedTargets != null)
pointsToAnalysisResult[o.Kind, o.Syntax]).ToImmutableArray()));
if (positivePointsToTaintedTargets.Any())
{
if (allTaintedTargets == null)
{
Expand All @@ -73,14 +81,20 @@ internal static class TaintedDataSymbolMapExtensions

foreach ((MethodMatcher methodMatcher, ImmutableHashSet<(ValueContentCheck valueContentCheck, string)> valueContentTaintedTargets) in sourceInfo.TaintedMethodsNeedsValueContentAnalysis)
{
if (methodMatcher(method.Name, arguments))
if (valueContentTaintedTargets.Any() && methodMatcher(method.Name, arguments))
{
pointsToAnalysisResult ??= valueContentFactory.Value.p;
valueContentAnalysisResult ??= valueContentFactory.Value.v;
if (pointsToAnalysisResult == null || valueContentAnalysisResult == null)
{
break;
}

IEnumerable<(ValueContentCheck, string target)> positiveValueContentTaintedTargets = valueContentTaintedTargets.Where(s =>
s.valueContentCheck(
arguments.Select(o =>
pointsToAnalysisResult.Value[o.Kind, o.Syntax]).ToImmutableArray(),
arguments.Select(o => valueContentAnalysisResult.Value[o.Kind, o.Syntax]).ToImmutableArray()));
if (positiveValueContentTaintedTargets != null)
arguments.Select(o => pointsToAnalysisResult[o.Kind, o.Syntax]).ToImmutableArray(),
arguments.Select(o => valueContentAnalysisResult[o.Kind, o.Syntax]).ToImmutableArray()));
if (positiveValueContentTaintedTargets.Any())
{
if (allTaintedTargets == null)
{
Expand Down

0 comments on commit dbc50b1

Please sign in to comment.