-
Notifications
You must be signed in to change notification settings - Fork 480
Use DefaultDllImportSearchPaths attribute #2658
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
Changes from 25 commits
bcaaa2a
e09d6ca
20ec3e0
2a7eecc
3207b5f
3543d04
c9e6108
314ec65
dd1ff21
03a4151
4eb57f0
67a066e
98f8ffc
b66bb54
80412bb
39324b8
b143627
c43e691
a873385
f620275
6ca96e7
d43709f
3704fb1
d71044c
b61330e
6d53be4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1074,4 +1074,22 @@ | |
| <data name="JsonNetMaybeInsecureSerializerTitle" xml:space="preserve"> | ||
| <value>Ensure that JsonSerializer has a secure configuration when deserializing</value> | ||
| </data> | ||
| <data name="UseDefaultDllImportSearchPathsAttribute" xml:space="preserve"> | ||
| <value>Use DefaultDllImportSearchPaths attribute for P/Invokes</value> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Trying to think of a title/message/description kinda makes me think we should have a separate rule for checking the value. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you thumbs up the comment, does that mean you're going to add another rule? 🙂
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NP. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that a "yes let's have a separate diagnostic if the value has unsafe bits", or a "no let's use the same diagnostic"? 🙂 |
||
| </data> | ||
| <data name="UseDefaultDllImportSearchPathsAttributeDescription" xml:space="preserve"> | ||
| <value>By default, P/Invokes using DllImportAttribute probe a number of directories, including the current working directory for the library to load. This can be a security issue for certain applications, leading to DLL hijacking.</value> | ||
| </data> | ||
| <data name="UseDefaultDllImportSearchPathsAttributeMessage" xml:space="preserve"> | ||
| <value>The method {0} didn't use DefaultDllImportSearchPaths attribute for P/Invokes.</value> | ||
| </data> | ||
| <data name="DoNotUseUnsafeDllImportSearchPath" xml:space="preserve"> | ||
| <value>Do not use unsafe DllImportSearchPath value</value> | ||
| </data> | ||
| <data name="DoNotUseUnsafeDllImportSearchPathDescription" xml:space="preserve"> | ||
| <value>There could be malicious DLL under the application directory or the default DLL search directories. Use an DllImportSearchPath value that sepecifies explicit search path instead</value> | ||
LLLXXXCCC marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| </data> | ||
| <data name="DoNotUseUnsafeDllImportSearchPathMessage" xml:space="preserve"> | ||
| <value>Use unsafe DllImportSearchPath value {0}</value> | ||
LLLXXXCCC marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| </data> | ||
| </root> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System.Collections.Immutable; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Runtime.InteropServices; | ||
| using Analyzer.Utilities; | ||
| using Analyzer.Utilities.Extensions; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
| using Microsoft.CodeAnalysis.FlowAnalysis.DataFlow; | ||
| using Microsoft.NetCore.Analyzers.Security.Helpers; | ||
|
|
||
| namespace Microsoft.NetCore.Analyzers.Security | ||
| { | ||
| [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] | ||
| public sealed class UseDefaultDllImportSearchPathsAttribute : DiagnosticAnalyzer | ||
| { | ||
| internal static DiagnosticDescriptor UseDefaultDllImportSearchPathsAttributeRule = SecurityHelpers.CreateDiagnosticDescriptor( | ||
| "CA5392", | ||
| typeof(MicrosoftNetCoreAnalyzersResources), | ||
| nameof(MicrosoftNetCoreAnalyzersResources.UseDefaultDllImportSearchPathsAttribute), | ||
| nameof(MicrosoftNetCoreAnalyzersResources.UseDefaultDllImportSearchPathsAttributeMessage), | ||
| DiagnosticHelpers.EnabledByDefaultIfNotBuildingVSIX, | ||
| helpLinkUri: null, | ||
| descriptionResourceStringName: nameof(MicrosoftNetCoreAnalyzersResources.UseDefaultDllImportSearchPathsAttributeDescription), | ||
| customTags: WellKnownDiagnosticTags.Telemetry); | ||
| internal static DiagnosticDescriptor DoNotUseUnsafeDllImportSearchPathRule = SecurityHelpers.CreateDiagnosticDescriptor( | ||
| "CA5393", | ||
| nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseUnsafeDllImportSearchPath), | ||
| nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseUnsafeDllImportSearchPathMessage), | ||
| DiagnosticHelpers.EnabledByDefaultIfNotBuildingVSIX, | ||
| helpLinkUri: null, | ||
| descriptionResourceStringName: nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseUnsafeDllImportSearchPathDescription), | ||
| customTags: WellKnownDiagnosticTagsExtensions.DataflowAndTelemetry); | ||
LLLXXXCCC marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // DllImportSearchPath.AssemblyDirectory = 2. | ||
| // DllImportSearchPath.UseDllDirectoryForDependencies = 256. | ||
| // DllImportSearchPath.ApplicationDirectory = 512. | ||
| private const int UnsafeBits = 2 | 256 | 512; | ||
| private const int LegacyBehavior = 0; | ||
|
|
||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create( | ||
| UseDefaultDllImportSearchPathsAttributeRule, | ||
| DoNotUseUnsafeDllImportSearchPathRule); | ||
|
|
||
| public override void Initialize(AnalysisContext context) | ||
| { | ||
| context.EnableConcurrentExecution(); | ||
|
|
||
| // Security analyzer - analyze and report diagnostics on generated code. | ||
| context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); | ||
|
|
||
| context.RegisterCompilationStartAction(compilationStartAnalysisContext => | ||
| { | ||
| var compilation = compilationStartAnalysisContext.Compilation; | ||
| var wellKnownTypeProvider = WellKnownTypeProvider.GetOrCreate(compilation); | ||
|
|
||
| if (!wellKnownTypeProvider.TryGetTypeByMetadataName(WellKnownTypeNames.SystemRuntimeInteropServicesDllImportAttribute, out INamedTypeSymbol dllImportAttributeTypeSymbol) || | ||
| !wellKnownTypeProvider.TryGetTypeByMetadataName(WellKnownTypeNames.SystemRuntimeInteropServicesDefaultDllImportSearchPathsAttribute, out INamedTypeSymbol defaultDllImportSearchPathsAttributeTypeSymbol)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var cancellationToken = compilationStartAnalysisContext.CancellationToken; | ||
| var unsafeDllImportSearchPathBits = compilationStartAnalysisContext.Options.GetUnsignedIntegralOptionValue( | ||
| optionName: EditorConfigOptionNames.UnsafeDllImportSearchPathBits, | ||
| rule: DoNotUseUnsafeDllImportSearchPathRule, | ||
| defaultValue: UnsafeBits, | ||
| cancellationToken: cancellationToken); | ||
| var defaultDllImportSearchPathsAttributeOnAssembly = compilation.Assembly.GetAttributes().FirstOrDefault(o => o.AttributeClass.Equals(defaultDllImportSearchPathsAttributeTypeSymbol)); | ||
|
|
||
| compilationStartAnalysisContext.RegisterSymbolAction(symbolAnalysisContext => | ||
| { | ||
| var symbol = symbolAnalysisContext.Symbol; | ||
|
|
||
| if (!symbol.IsExtern || !symbol.IsStatic) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var dllImportAttribute = symbol.GetAttributes().FirstOrDefault(s => s.AttributeClass.Equals(dllImportAttributeTypeSymbol)); | ||
| var defaultDllImportSearchPathsAttribute = symbol.GetAttributes().FirstOrDefault(s => s.AttributeClass.Equals(defaultDllImportSearchPathsAttributeTypeSymbol)); | ||
|
|
||
| if (dllImportAttribute != null) | ||
| { | ||
| var constructorArguments = dllImportAttribute.ConstructorArguments; | ||
|
|
||
| if (constructorArguments.Length == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (Path.IsPathRooted(constructorArguments[0].Value.ToString())) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var rule = UseDefaultDllImportSearchPathsAttributeRule; | ||
| var ruleArgument = symbol.Name; | ||
|
|
||
| if (defaultDllImportSearchPathsAttribute == null) | ||
| { | ||
| if (defaultDllImportSearchPathsAttributeOnAssembly != null) | ||
| { | ||
| var dllImportSearchPathOnAssembly = (int)defaultDllImportSearchPathsAttributeOnAssembly.ConstructorArguments.FirstOrDefault().Value; | ||
| var validBits = dllImportSearchPathOnAssembly & unsafeDllImportSearchPathBits; | ||
|
|
||
| if (dllImportSearchPathOnAssembly != LegacyBehavior && | ||
| validBits == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| rule = DoNotUseUnsafeDllImportSearchPathRule; | ||
| ruleArgument = ((DllImportSearchPath)validBits).ToString(); | ||
| } | ||
|
||
| } | ||
| else | ||
| { | ||
| var dllImportSearchPath = (int)defaultDllImportSearchPathsAttribute.ConstructorArguments.FirstOrDefault().Value; | ||
| var validBits = dllImportSearchPath & unsafeDllImportSearchPathBits; | ||
|
|
||
| if (dllImportSearchPath != LegacyBehavior && | ||
| validBits == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| rule = DoNotUseUnsafeDllImportSearchPathRule; | ||
| ruleArgument = ((DllImportSearchPath)validBits).ToString(); | ||
| } | ||
|
|
||
| symbolAnalysisContext.ReportDiagnostic( | ||
| symbol.CreateDiagnostic( | ||
| rule, | ||
| ruleArgument)); | ||
| } | ||
| }, SymbolKind.Method); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.