Skip to content

Commit

Permalink
Enable NETAnalyzer (#337)
Browse files Browse the repository at this point in the history
* Enable NETAnalyzer

* fixing dotnet-format issuee
  • Loading branch information
eddynaka authored Jan 21, 2021
1 parent d912372 commit 9a0d252
Show file tree
Hide file tree
Showing 42 changed files with 41 additions and 68 deletions.
2 changes: 1 addition & 1 deletion src/BinSkim.Driver/DumpCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private void DumpFile(string target, bool verbose)
}

string language = pe.IsManaged ? "Pure Managed" : "Native";
if (pe.IsManaged && !pe.IsILOnly) { language = "Mixed Managed"; };
if (pe.IsManaged && !pe.IsILOnly) { language = "Mixed Managed"; }
sb.Append(language);

string machine = pe.Machine.ToString();
Expand Down
2 changes: 1 addition & 1 deletion src/BinSkim.Driver/RoslynAnalyzer/ActionMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.CodeAnalysis.IL
{
/// <summary>
/// A map of categorized actions that can be sequentially retrieved
/// and invoked by providing a relevant key and context object,
/// and invoked by providing a relevant key and context object,
/// serving as a kind of multicast delegate mechanism.
/// </summary>
/// <typeparam name="TContext">A context instance that is passed to the action.</typeparam>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace Microsoft.CodeAnalysis.IL
{
/// <summary>
/// Basic analysis context provided to Roslyn analyzers. We use a singleton instance of this class to
/// Basic analysis context provided to Roslyn analyzers. We use a singleton instance of this class to
/// capture all symbol action registration for analyzers. These actions will subsequently be invoked
/// as we visit the IL of all analysis targets.
/// </summary>
Expand Down Expand Up @@ -40,7 +40,7 @@ public override void RegisterSymbolAction(Action<SymbolAnalysisContext> action,

/// <summary>
/// Register a compilation end action. We record and invoke these callbacks after analyzing each assembly, in order
/// to allow analyzers to perform any relevant clean-up actions.
/// to allow analyzers to perform any relevant clean-up actions.
/// </summary>
/// <param name="action"></param>
public override void RegisterCompilationEndAction(Action<CompilationAnalysisContext> action)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ public class DoNotMarkStackAsExecutable : ELFBinarySkimmerBase
public override string Id => RuleIds.DoNotMarkStackAsExecutable;

/// <summary>
/// "This checks if a binary has an executable stack; an
/// executable stack allows attackers to redirect code flow
/// into stack memory, which is an easy place for an attacker
/// "This checks if a binary has an executable stack; an
/// executable stack allows attackers to redirect code flow
/// into stack memory, which is an easy place for an attacker
/// to store shellcode. Ensure you are compiling with '-z noexecstack'
/// to mark the stack as non-executable."
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ public class EnableReadOnlyRelocations : ELFBinarySkimmerBase
public override string Id => RuleIds.EnableReadOnlyRelocations;

/// <summary>
/// This check ensures that some relocation data is marked as read only after
/// the executable is loaded, and moved below the .data section in memory.
/// This prevents them from being overwritten, which can redirect control flow.
/// This check ensures that some relocation data is marked as read only after
/// the executable is loaded, and moved below the .data section in memory.
/// This prevents them from being overwritten, which can redirect control flow.
/// Use the compiler flags '-Wl,z,relro' to enable this.
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA3010_EnableReadOnlyRelocations_Description };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public override AnalysisApplicability CanAnalyzeElf(ELFBinary target, Sarif.Prop

/// <summary>
/// Checks if Fortified functions are used--the -DFORTIFY_SOURCE=2 flag enables these when -O2 is enabled.
///
///
/// Check implementation:
/// -Get all function symbols in the ELF binary
/// -Check for any fortified functions--if we find any, we used the option.
Expand Down
1 change: 0 additions & 1 deletion src/BinSkim.Rules/ELFRules/ELFBinarySkimmerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,4 @@ public sealed override AnalysisApplicability CanAnalyze(BinaryAnalyzerContext co

public abstract AnalysisApplicability CanAnalyzeElf(ELFBinary target, Sarif.PropertiesDictionary policy, out string reasonForNotAnalyzing);
}

}
3 changes: 0 additions & 3 deletions src/BinSkim.Rules/NativeInterop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ internal struct CERT_CHAIN_ELEMENT
internal IntPtr pwszExtendedErrorInfo;
}


[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
internal struct CERT_CONTEXT
{
Expand Down Expand Up @@ -251,7 +250,6 @@ internal static bool CryptHashPublicKeyInfoWrapper(
return CryptHashPublicKeyInfo(hCryptProv, Algid, dwFlags, dwCertEncodingType, ref pInfo, pbComputedHash, ref pcbComputedHash);
}


[DllImport("crypt32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
private static extern
bool CryptHashToBeSigned(
Expand Down Expand Up @@ -279,7 +277,6 @@ internal static bool CryptHashToBeSignedWrapper(
ref pcbComputedHash);
}


[DllImport("wintrust.dll")]
private static extern uint WinVerifyTrust(
IntPtr hwnd,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ public class LoadImageAboveFourGigabyteAddress : PEBinarySkimmerBase
/// effectiveness at mitigating memory corruption vulnerabilities. To resolve
/// this issue, either use the default preferred base address by removing any
/// uses of /baseaddress from compiler command lines, or /BASE from linker
/// command lines (recommended), or configure your program to start at a base
/// command lines (recommended), or configure your program to start at a base
/// address above 4GB when compiled for 64 bit platforms (by changing the
/// constant passed to /baseaddress / /BASE). Note that if you choose to
/// continue using a custom preferred base address, you will need to make this
/// modification only for 64-bit builds, as base addresses above 4GB are not
/// valid for 32-bit binaries.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2001_LoadImageAboveFourGigabyteAddress_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ private static StringToVersionMap BuildDefaultVulnerableBinariesMap()
// Between one and unlimited times, as many times as possible, giving back as needed (greedy) «+»
private static readonly Regex s_versionRegex = new Regex(@"\d+(\.\d+){0,3}", RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture);


public override AnalysisApplicability CanAnalyzePE(PEBinary target, Sarif.PropertiesDictionary policy, out string reasonForNotAnalyzing)
{
reasonForNotAnalyzing = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public class EnableCriticalCompilerWarnings : WindowsBinaryAndPdbSkimmerBase, IO
/// level 3 or higher by supplying /W3, /W4, or /Wall to the compiler, and
/// resolve the warnings emitted.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2007_EnableCriticalCompilerWarnings_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
1 change: 0 additions & 1 deletion src/BinSkim.Rules/PERules/BA2008.EnableControlFlowGuard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public class EnableControlFlowGuard : PEBinarySkimmerBase, IOptionsProvider
/// target is an expected, safe location. If that check fails at runtime,
/// the operating system will close the program.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2008_EnableControlFlowGuard_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public class EnableAddressSpaceLayoutRandomization : PEBinarySkimmerBase
/// linker command line. For .NET applications, use a compiler shipping with
/// Visual Studio 2008 or later.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2009_EnableAddressSpaceLayoutRandomization_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down Expand Up @@ -103,7 +102,6 @@ public override void Analyze(BinaryAnalyzerContext context)

bool relocSectionFound = false;


// For WinCE 7+ ASLR is a machine-wide setting and binaries must
// have relocation info present in order to be dynamically rebased.
foreach (SectionHeader sectionHeader in target.PE.PEHeaders.SectionHeaders)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public class DoNotMarkImportsSectionAsExecutable : PEBinarySkimmerBase
/// segment in source code, which change the imports section to be executable, or
/// which merge the ".rdata" segment into an executable section.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2010_DoNotMarkImportsSectionAsExecutable_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class DoNotModifyStackProtectionCookie : PEBinarySkimmerBase
/// reference or create a symbol named __security_cookie or
/// __security_cookie_complement.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2012_DoNotModifyStackProtectionCookie_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down Expand Up @@ -145,7 +144,6 @@ private bool Validate64BitImage(BinaryAnalyzerContext context)
SafePointer fileCookiePtr = loadConfigVA;
fileCookiePtr.Address = (int)fileCookieOffset;


SafePointer boundsCheck = fileCookiePtr + 8;
if (!this.CookieOffsetValid(context, boundsCheck))
{
Expand All @@ -168,7 +166,6 @@ private bool Validate64BitImage(BinaryAnalyzerContext context)
return true;
}


private bool Validate32BitImage(BinaryAnalyzerContext context)
{
PEBinary target = context.PEBinary();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class InitializeStackProtection : WindowsBinaryAndPdbSkimmerBase
{
/// <summary>
/// BA2013
///
///
/// </summary>
public override string Id => RuleIds.InitializeStackProtection;

Expand All @@ -35,7 +35,6 @@ public class InitializeStackProtection : WindowsBinaryAndPdbSkimmerBase
/// this call for you, or call __security_init_cookie() manually in your
/// custom entry point.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2013_InitializeStackProtection_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class DoNotDisableStackProtectionForFunctions : WindowsBinaryAndPdbSkimme
public override string Id => RuleIds.DoNotDisableStackProtectionForFunctions;

/// <summary>
/// Application code should not disable stack protection for individual functions.
/// Application code should not disable stack protection for individual functions.
/// The stack protector (/GS) is a security feature of the Windows native compiler
/// which makes it more difficult to exploit stack buffer overflow memory corruption
/// vulnerabilities. Disabling the stack protector, even on a function-by-function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public class EnableHighEntropyVirtualAddresses : PEBinarySkimmerBase
/// Binaries must also be compiled as /LARGEADDRESSAWARE in order to enable
/// high entropy ASLR.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2015_EnableHighEntropyVirtualAddresses_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
3 changes: 1 addition & 2 deletions src/BinSkim.Rules/PERules/BA2016.MarkImageAsNXCompatible.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,9 @@ public class MarkImageAsNXCompatible : PEBinarySkimmerBase
/// direct shellcode in their exploit (because the exploit comes in the
/// form of input data to the exploited program on a data segment,
/// rather than on an executable code segment). Ensure that your tool
/// chain is configured to mark your binaries as NX compatible, e.g. by
/// chain is configured to mark your binaries as NX compatible, e.g. by
/// passing /NXCOMPAT to the C/C++ linker.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2016_MarkImageAsNXCompatible_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
1 change: 0 additions & 1 deletion src/BinSkim.Rules/PERules/BA2018.EnableSafeSEH.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public class EnableSafeSEH : PEBinarySkimmerBase
/// configure your build system to supply this flag for x86 builds only,
/// as the /SafeSEH flag is invalid when linking for ARM and x64.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2018_EnableSafeSEH_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class DoNotMarkWritableSectionsAsShared : PEBinarySkimmerBase
/// section with both attributes. Be sure to disable incremental linking in release
/// builds, as this feature creates a writable and executable section named '.textbss'
/// in order to function.
/// </summary>
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2019_DoNotMarkWritableSectionsAsShared_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public class DoNotMarkWritableSectionsAsExecutable : PEBinarySkimmerBase
/// linker command line for C and C++ programs, or #pragma section in C and C++
/// source code, which mark a section with both attributes.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2021_DoNotMarkWritableSectionsAsExecutable_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
5 changes: 2 additions & 3 deletions src/BinSkim.Rules/PERules/BA2022.SignSecurely.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ public class SignSecurely : WindowsBinarySkimmerBase
public override string Id => RuleIds.SignSecurely;

/// <summary>
/// Images should be correctly signed by trusted publishers using
/// cryptographically secure signature algorithms. This rule
/// Images should be correctly signed by trusted publishers using
/// cryptographically secure signature algorithms. This rule
/// invokes WinTrustVerify to validate that binary hash, signing
/// and public key algorithms are secure and, where configurable,
/// that key sizes meet acceptable size thresholds.
/// </summary>

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA2022_SignCorrectly_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
Expand Down
6 changes: 3 additions & 3 deletions src/BinSkim.Rules/PERules/BA2024.EnableSpectreMitigations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ internal static Version GetClosestCompilerVersionWithSpectreMitigations(BinaryAn
}

/// <summary>
/// Get the Spectre compiler compiler mitigations available for a particular compiler version and machine type.
/// Get the Spectre compiler mitigations available for a particular compiler version and machine type.
/// </summary>
internal static CompilerMitigations GetAvailableMitigations(BinaryAnalyzerContext context, ExtendedMachine machine, Version omVersion)
{
Expand Down Expand Up @@ -528,8 +528,8 @@ A servicing update to Visual Studio 2015 Update 3
armData.Add("19.14.26329.0 - *.*.*.*",
(CompilerMitigations.D2GuardSpecLoadAvailable | CompilerMitigations.QSpectreAvailable | CompilerMitigations.NonoptimizedCodeMitigated).ToString());

compilersData.Add(MachineFamily.X86.ToString(), x86Data);
compilersData.Add(MachineFamily.Arm.ToString(), armData);
compilersData.Add(nameof(MachineFamily.X86), x86Data);
compilersData.Add(nameof(MachineFamily.Arm), armData);

return compilersData;
}
Expand Down
1 change: 0 additions & 1 deletion src/BinSkim.Rules/ReproInformationConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ public static class ReproInformationConfiguration
internal static PerLanguageOption<StringToVersionMap> BA2024_AllowedLibraries { get; } =
new PerLanguageOption<StringToVersionMap>(
null, nameof(BA2024_AllowedLibraries), defaultValue: () => null);

}
}
1 change: 0 additions & 1 deletion src/BinSkim.Rules/VulnerableDependencyDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ internal class VulnerableDependencyDescriptor : PropertiesDictionary
{
public VulnerableDependencyDescriptor()
{

}

public VulnerableDependencyDescriptor(PropertiesDictionary dictionary = null)
Expand Down
1 change: 0 additions & 1 deletion src/BinSkim.Sdk/BuildKind.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.


namespace Microsoft.CodeAnalysis.IL.Sdk
{
public enum BuildKind
Expand Down
2 changes: 1 addition & 1 deletion src/BinaryParsers/ELFBinary/ELFCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public enum ELFCompilerType
/// <summary>
/// Many compilers on Linux note which compiler and version was used to build that code
/// by adding an entry to the '.comment' section (represented as a series of null terminated ascii strings).
///
///
/// This class takes those strings and attempts to match them with a known toolchain & version.
/// </summary>
public class ELFCompiler
Expand Down
3 changes: 1 addition & 2 deletions src/BinaryParsers/PEBinary/PortableExecutable/ImageHeader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public object GetField(int n)
object res;
ImageFieldData fi = this.GetFieldInfo(n);
int count = fi.Count;
int offset = this.GetFieldOffset(n); ;
int offset = this.GetFieldOffset(n);
object o;
int len;

Expand Down Expand Up @@ -81,7 +81,6 @@ public int GetFieldSize(int n)
int size;
int len;


if (fi.VarLen)
{
SafePointer field_start = this.m_pHeader + this.GetFieldOffset(n);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ public class ImageLoadConfigDirectory64 : ImageHeader

public ImageLoadConfigDirectory64(PEHeader parentHeader, SafePointer sp) : base(parentHeader, sp) { }


protected override ImageFieldData[] GetFields() { return s_fields; }

public override ImageHeader Create(PEHeader parentHeader, SafePointer sp)
Expand Down
3 changes: 1 addition & 2 deletions src/BinaryParsers/PEBinary/PortableExecutable/PE.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ public void Dispose()
}
}


public Exception LoadException { get; set; }

public Uri Uri { get; set; }
Expand Down Expand Up @@ -190,7 +189,7 @@ public string[] Imports
DirectoryEntry importTableDirectory = this.PEHeaders.PEHeader.ImportTableDirectory;
if (this.PEHeaders.PEHeader.ImportTableDirectory.Size == 0)
{
this.asImports = new string[0];
this.asImports = Array.Empty<string>();
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void AppendString(StringBuilder sb)
sb.Append(this.Object);
sb.Append(" (");
sb.Append(this.Library);
sb.Append(")");
sb.Append(')');
}

if (!string.IsNullOrWhiteSpace(this.Suffix))
Expand Down
Loading

0 comments on commit 9a0d252

Please sign in to comment.