Skip to content
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

BinSkim .NET Updates to version 9 #1024

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ jobs:
- name: Setup .NET SDK
uses: actions/setup-dotnet@v4
with:
dotnet-version: 3.1.x
- name: Setup .NET SDK
uses: actions/setup-dotnet@v4
with:
dotnet-version: 8.0.x
dotnet-version: 9.0.x

- name: Checkout repository
uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dotnet-format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ jobs:
run: dotnet tool install -g dotnet-format

- name: dotnet format
run: dotnet-format --folder --check
run: dotnet-format --folder --check
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: is this intentional?

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ x64/

# Workaround for an msbuild/dotnet bug on Linux
src/BinSkimLinux.sln
.vscode/settings.json
35 changes: 16 additions & 19 deletions BuildAndTest.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ call SetCurrentVersion.cmd
set VERSION_CONSTANTS=%~dp0src\BinaryParsers\VersionConstants.cs

@REM Rewrite VersionConstants.cs
echo // Copyright (c) Microsoft. All rights reserved. Licensed under the MIT > %VERSION_CONSTANTS%
echo // license. See LICENSE file in the project root for full license information. >> %VERSION_CONSTANTS%
echo namespace Microsoft.CodeAnalysis.IL >> %VERSION_CONSTANTS%
echo { >> %VERSION_CONSTANTS%
echo public static class VersionConstants >> %VERSION_CONSTANTS%
echo { >> %VERSION_CONSTANTS%
echo public const string Prerelease = "%PRERELEASE%"; >> %VERSION_CONSTANTS%
echo public const string AssemblyVersion = "%MAJOR%.%MINOR%.%PATCH%" + ".0"; >> %VERSION_CONSTANTS%
echo public const string FileVersion = "%MAJOR%.%MINOR%.%PATCH%" + ".0"; >> %VERSION_CONSTANTS%
echo public const string Version = AssemblyVersion + Prerelease; >> %VERSION_CONSTANTS%
echo } >> %VERSION_CONSTANTS%
echo } >> %VERSION_CONSTANTS%
echo // Copyright (c) Microsoft. All rights reserved. Licensed under the MIT> %VERSION_CONSTANTS%
echo // license. See LICENSE file in the project root for full license information.>> %VERSION_CONSTANTS%
echo namespace Microsoft.CodeAnalysis.IL>> %VERSION_CONSTANTS%
echo {>> %VERSION_CONSTANTS%
echo public static class VersionConstants>> %VERSION_CONSTANTS%
echo {>> %VERSION_CONSTANTS%
echo public const string Prerelease = "%PRERELEASE%";>> %VERSION_CONSTANTS%
echo public const string AssemblyVersion = "%MAJOR%.%MINOR%.%PATCH%" + ".0";>> %VERSION_CONSTANTS%
echo public const string FileVersion = "%MAJOR%.%MINOR%.%PATCH%" + ".0";>> %VERSION_CONSTANTS%
echo public const string Version = AssemblyVersion + Prerelease;>> %VERSION_CONSTANTS%
echo }>> %VERSION_CONSTANTS%
echo }>> %VERSION_CONSTANTS%


::Restore packages
Expand All @@ -63,12 +63,9 @@ call :RunTestProject BinSkim.Rules Functional || goto :ExitFailed

::Create the BinSkim platform specific publish packages
echo Creating Platform Specific BinSkim 'Publish' Packages
call :CreatePublishPackage netcoreapp3.1 win-x64 || goto :ExitFailed
call :CreatePublishPackage netcoreapp3.1 linux-x64 || goto :ExitFailed
call :CreatePublishPackage netcoreapp3.1 osx-x64 || goto :ExitFailed
call :CreatePublishPackage net6.0 win-x64 || goto :ExitFailed
call :CreatePublishPackage net6.0 linux-x64 || goto :ExitFailed
call :CreatePublishPackage net6.0 osx-x64 || goto :ExitFailed
call :CreatePublishPackage net9.0 win-x64 || goto :ExitFailed
call :CreatePublishPackage net9.0 linux-x64 || goto :ExitFailed
call :CreatePublishPackage net9.0 osx-x64 || goto :ExitFailed

::Build NuGet package
echo BuildPackages.cmd
Expand All @@ -79,7 +76,7 @@ dotnet tool update --global dotnet-format

::Update BinSkimRules.md to cover any xml changes
echo Exporting any BinSkim rules
.\bld\bin\x64_Release\netcoreapp3.1\BinSkim.exe export-rules .\docs\BinSkimRules.md
.\bld\bin\x64_Release\net9.0\BinSkim.exe export-rules .\docs\BinSkimRules.md

goto :Exit

Expand Down
8 changes: 4 additions & 4 deletions BuildAndTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fi

dotnet build src/BinSkimUnix.sln --configuration Release /p:Platform="x64"

dotnet test bld/bin/x64_Release/netcoreapp3.1/Test.FunctionalTests.BinSkim.Driver.dll
dotnet test bld/bin/x64_Release/netcoreapp3.1/Test.FunctionalTests.BinSkim.Rules.dll
dotnet test bld/bin/x64_Release/netcoreapp3.1/Test.UnitTests.BinaryParsers.dll
dotnet test bld/bin/x64_Release/netcoreapp3.1/Test.UnitTests.BinSkim.Rules.dll
dotnet test bld/bin/x64_Release/net9.0/Test.FunctionalTests.BinSkim.Driver.dll
dotnet test bld/bin/x64_Release/net9.0/Test.FunctionalTests.BinSkim.Rules.dll
dotnet test bld/bin/x64_Release/net9.0/Test.UnitTests.BinaryParsers.dll
dotnet test bld/bin/x64_Release/net9.0/Test.UnitTests.BinSkim.Rules.dll
50 changes: 7 additions & 43 deletions ado-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,12 @@ jobs:
pool:
vmImage: "ubuntu-20.04"
steps:
- task: UseDotNet@2
displayName: .NET Core 3.1 sdk
inputs:
version: "3.1.x"
packageType: sdk

- task: UseDotNet@2
displayName: .NET Core 6.0 sdk
inputs:
version: "6.0.x"
packageType: sdk

- checkout: self

- task: UseDotNet@2
displayName: .NET Core 8 sdk
displayName: .NET Core 9 sdk
inputs:
version: "8.0.x"
version: "9.0.x"
packageType: sdk

- task: Bash@3
Expand All @@ -37,21 +25,9 @@ jobs:
vmImage: "windows-latest"
steps:
- task: UseDotNet@2
displayName: .NET Core 3.1 sdk
inputs:
version: "3.1.x"
packageType: sdk

- task: UseDotNet@2
displayName: .NET Core 6.0 sdk
displayName: .NET Core 9 sdk
inputs:
version: "6.0.x"
packageType: sdk

- task: UseDotNet@2
displayName: .NET Core 8 sdk
inputs:
version: "8.0.x"
version: "9.0.x"
packageType: sdk

- checkout: self
Expand All @@ -68,21 +44,9 @@ jobs:
vmImage: "macOS-latest"
steps:
- task: UseDotNet@2
displayName: .NET Core 3.1 sdk
inputs:
version: "3.1.x"
packageType: sdk

- task: UseDotNet@2
displayName: .NET Core 6.0 sdk
inputs:
version: "6.0.x"
packageType: sdk

- task: UseDotNet@2
displayName: .NET Core 8 sdk
displayName: .NET Core 9 sdk
inputs:
version: "8.0.x"
version: "9.0.x"
packageType: sdk

- checkout: self
Expand All @@ -97,4 +61,4 @@ jobs:
displayName: "Run BinSkim"
inputs:
targetType: "inline"
script: "dotnet bld/bin/x64_Release/netcoreapp3.1/binskim.dll analyze src/Test.FunctionalTests.BinSkim.Driver/BaselineTestData/macho.*"
script: "dotnet bld/bin/x64_Release/net9.0/binskim.dll analyze src/Test.FunctionalTests.BinSkim.Driver/BaselineTestData/macho.*"
9 changes: 0 additions & 9 deletions global.json

This file was deleted.

2 changes: 1 addition & 1 deletion scripts/SimpleStressTest.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
[CmdletBinding()]
param(
[string]
$BinSkimFolder = "..\bld\bin\x64_Release\netcoreapp3.1",
$BinSkimFolder = "..\bld\bin\x64_Release\net9.0",

[string]
$SessionName = "stress",
Expand Down
2 changes: 1 addition & 1 deletion src/BinSkim.Rules/BinSkim.Rules.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., build.netcore.props))\build.netcore.props" />
<PropertyGroup>
<RootNamespace>Microsoft.CodeAnalysis.IL.Rules</RootNamespace>
<TargetFramework>$(NetStandardVersion)</TargetFramework>
<TargetFramework>$(NetCoreVersion)</TargetFramework>
<Platforms>x64</Platforms>
</PropertyGroup>
<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/BinSkim.Sdk/BinSkim.Sdk.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., build.netcore.props))\build.netcore.props" />
<PropertyGroup>
<RootNamespace>Microsoft.CodeAnalysis.IL.Sdk</RootNamespace>
<TargetFramework>$(NetStandardVersion)</TargetFramework>
<TargetFramework>$(NetCoreVersion)</TargetFramework>
<Platforms>x64</Platforms>
</PropertyGroup>
<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/BinaryParsers/BinaryParsers.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., build.netcore.props))\build.netcore.props" />
<PropertyGroup>
<RootNamespace>Microsoft.CodeAnalysis.BinaryParsers</RootNamespace>
<TargetFramework>$(NetStandardVersion)</TargetFramework>
<TargetFramework>$(NetCoreVersion)</TargetFramework>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Platforms>x64</Platforms>
</PropertyGroup>
Expand Down
15 changes: 10 additions & 5 deletions src/BinaryParsers/ElfBinary/Dwarf/DwarfMemoryReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,18 @@
/// <summary>
/// Reads the string from the current position in the stream.
/// </summary>
[HandleProcessCorruptedStateExceptions]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be reading the docs wrong, but with HandleProcessCorruptedStateExceptions being removed - are you still trying to catch CSEs with the try-catch block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I went over the docs, and as of now the only option seems to be try catch, so I did it with the generic try catch clause. But I'm more than open to the suggestions for improvements :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm kind of wondering if try-catch here actually serves any purpose and whether we can remove it altogether to not give us a false sense of "security" of exception being caught (from docs):

By default, the common language runtime (CLR) does not deliver these exceptions to managed code, and the try/catch blocks (and other exception-handling clauses) are not invoked for them.

.NET Core only: Even though this attribute exists in .NET Core, since the recovery from corrupted process state exceptions is not supported, this attribute is ignored. The CLR doesn't deliver corrupted process state exceptions to the managed code.

It doesn't seem like we have any automated tests for this right?

public string ReadString()
{
string result = Marshal.PtrToStringAnsi(pointer + Position);

Position += result.Length + 1;
return result;
try
{
string result = Marshal.PtrToStringAnsi(pointer + Position);
Position += result.Length + 1;
return result;
}
catch (Exception ex)
{
throw new InvalidOperationException("Failed to read string from memory.", ex);
}
Comment on lines +132 to +135

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ private static void CoCreateFromMsdia(Guid clsidOfServer, Guid riid, out IntPtr
{
throw new InvalidOperationException("Could not get class object.");
}
var classFactory = (IClassFactory)Marshal.GetObjectForIUnknown(pClassFactory);
var classFactory = (IClassFactory)ResourceReleaser.GetObjectForIUnknown(pClassFactory);
classFactory.CreateInstance(IntPtr.Zero, ref riid, out pvObject);
Marshal.Release(pClassFactory);
Marshal.ReleaseComObject(classFactory);
ResourceReleaser.Release(classFactory);
}

private const string IDiaDataSourceRiid = "79F1BB5F-B66E-48E5-B6A9-1545C323CA3D";
Expand All @@ -44,7 +44,7 @@ public static IDiaDataSource GetDiaSource()
{
IntPtr diaSourcePtr = IntPtr.Zero;
CoCreateFromMsdia(new Guid(DiaSourceClsid), new Guid(IDiaDataSourceRiid), out diaSourcePtr);
object objectForIUnknown = Marshal.GetObjectForIUnknown(diaSourcePtr);
object objectForIUnknown = ResourceReleaser.GetObjectForIUnknown(diaSourcePtr);
var diaSourceInstance = objectForIUnknown as IDiaDataSource;
return diaSourceInstance;
}
Expand Down
20 changes: 10 additions & 10 deletions src/BinaryParsers/PEBinary/ProgramDatabase/Pdb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private IEnumerable<SourceFile> CreateSourceFileIteratorImpl(IDiaSymbol inObject
{
if (sourceFilesEnum != null)
{
Marshal.ReleaseComObject(sourceFilesEnum);
ResourceReleaser.Release(sourceFilesEnum);
}
}
}
Expand Down Expand Up @@ -241,7 +241,7 @@ private T CreateDiaTable<T>() where T : class
IDiaTable table = enumTables.Item(i);
if (!(table is T result))
{
Marshal.ReleaseComObject(table);
ResourceReleaser.Release(table);
}
else
{
Expand All @@ -253,7 +253,7 @@ private T CreateDiaTable<T>() where T : class
{
if (enumTables != null)
{
Marshal.ReleaseComObject(enumTables);
ResourceReleaser.Release(enumTables);
}
}

Expand Down Expand Up @@ -288,15 +288,15 @@ private HashSet<uint> GenerateWritableSegmentSet()
}
finally
{
Marshal.ReleaseComObject(segment);
ResourceReleaser.Release(segment);
}
}
}
finally
{
if (enumSegments != null)
{
Marshal.ReleaseComObject(enumSegments);
ResourceReleaser.Release(enumSegments);
}
}

Expand Down Expand Up @@ -341,15 +341,15 @@ private HashSet<uint> GenerateExecutableSectionContribIds()
}
finally
{
Marshal.ReleaseComObject(sectionContrib);
ResourceReleaser.Release(sectionContrib);
}
}
}
finally
{
if (enumSectionContribs != null)
{
Marshal.ReleaseComObject(enumSectionContribs);
ResourceReleaser.Release(enumSectionContribs);
}
}
return result;
Expand Down Expand Up @@ -397,12 +397,12 @@ public void Dispose()

if (this.session != null)
{
Marshal.ReleaseComObject(this.session);
ResourceReleaser.Release(this.session);
}

if (this.dataSource != null)
{
Marshal.ReleaseComObject(this.dataSource);
ResourceReleaser.Release(this.dataSource);
}
}

Expand Down Expand Up @@ -463,7 +463,7 @@ private void WindowsNativeLoadPdbFromPEUsingDia(string peOrPdbPath, string symbo
Environment.SetEnvironmentVariable("_NT_SYMBOL_PATH", "");
Environment.SetEnvironmentVariable("_NT_ALT_SYMBOL_PATH", "");

object pCallback = this.loadTrace != null ? this : (object)IntPtr.Zero;
object pCallback = this.loadTrace != null ? this : nint.Zero;

if (!string.IsNullOrEmpty(localSymbolDirectories))
{
Expand Down
30 changes: 30 additions & 0 deletions src/BinaryParsers/PEBinary/ProgramDatabase/ResourceReleaser.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Runtime.InteropServices;

namespace Microsoft.CodeAnalysis.BinaryParsers.ProgramDatabase
{
public static class ResourceReleaser
{
public static void Release(object resource)
{
if (resource != null && Marshal.IsComObject(resource) && RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Marshal.ReleaseComObject(resource);
}
else if (resource is IDisposable disposable)
{
disposable.Dispose();
}
}

public static object GetObjectForIUnknown(nint pointer)
{
return RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
? Marshal.GetObjectForIUnknown(pointer)
: new object();
}
}
}
2 changes: 1 addition & 1 deletion src/BinaryParsers/PEBinary/ProgramDatabase/SourceFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void Dispose()
{
if (!this.disposed)
{
Marshal.ReleaseComObject(this.sourceFile);
ResourceReleaser.Release(this.sourceFile);
}

this.disposed = true;
Expand Down
Loading
Loading