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

Creates BA3011 check for presence of BIND_NOW flag AKA full RELRO #363

Merged
merged 6 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/BinSkimRules.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,28 @@ The GNU_RELRO segment is missing from this binary, so relocation sections in '{0

---

## Rule `BA3011.EnableBindNow`

### Description

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,now' to enable this.

### Messages

#### `Pass`: Pass

The BIND_NOW flag was present, so '{0}' is protected.

#### `Error`: Error

The BIND_NOW flag is missing from this binary, so relocation sections in '{0}' will not be marked as read only after the binary is loaded. An attacker can overwrite these to redirect control flow. Ensure you are compiling with the compiler flags '-Wl,z,now' to address this.

#### `InvalidMetadata`: NotApplicable

'{0}' was not evaluated for check '{1}' as the analysis is not relevant based on observed metadata: {2}.

---

## Rule `BA3030.UseCheckedFunctionsWithGcc`

### Description
Expand Down
111 changes: 111 additions & 0 deletions src/BinSkim.Rules/ELFRules/BA3011.EnableBindNow.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// 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.Collections.Generic;
using System.Composition;

using ELFSharp.ELF;

using Microsoft.CodeAnalysis.BinaryParsers;
using Microsoft.CodeAnalysis.IL.Sdk;
using Microsoft.CodeAnalysis.Sarif;
using Microsoft.CodeAnalysis.Sarif.Driver;

namespace Microsoft.CodeAnalysis.IL.Rules
{
[Export(typeof(Skimmer<BinaryAnalyzerContext>)), Export(typeof(ReportingDescriptor))]
public class EnableBindNow : ELFBinarySkimmerBase
{
private const uint DF_1_NOW = 0x1;
private const uint DF_BIND_NOW = 0x8;

/// <summary>
/// BA3011
/// </summary>
public override string Id => RuleIds.EnableBindNow;

/// <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.
/// Use the compiler flags '-Wl,z,now' to enable this.
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA3011_EnableBindNow_Description };

protected override IEnumerable<string> MessageResourceNames => new string[] {
nameof(RuleResources.BA3011_Pass),
nameof(RuleResources.BA3011_Error),
nameof(RuleResources.NotApplicable_InvalidMetadata)
};

public override AnalysisApplicability CanAnalyzeElf(ELFBinary target, Sarif.PropertiesDictionary policy, out string reasonForNotAnalyzing)
{
IELF elf = target.ELF;

if (elf.Type == FileType.Core || elf.Type == FileType.None || elf.Type == FileType.Relocatable)
{
reasonForNotAnalyzing = MetadataConditions.ElfIsCoreNoneOrObject;
return AnalysisApplicability.NotApplicableToSpecifiedTarget;
}

reasonForNotAnalyzing = null;
return AnalysisApplicability.ApplicableToSpecifiedTarget;
}

public override void Analyze(BinaryAnalyzerContext context)
{
IELF elf = context.ELFBinary().ELF;

if (HasBindNowFlag(elf))
{
// Pass
context.Logger.Log(this,
RuleUtilities.BuildResult(ResultKind.Pass, context, null,
nameof(RuleResources.BA3011_Pass),
context.TargetUri.GetFileName()));
}
else
{
// Fail
context.Logger.Log(this,
RuleUtilities.BuildResult(FailureLevel.Error, context, null,
nameof(RuleResources.BA3011_Error),
context.TargetUri.GetFileName()));
}
}

// Pwntools checksec calls out numerous ways an ELF can specify full RELRO to the
// loader. All have the intended effect of eagerly resolving all plt functions and
// subsequently marking the entire GOT read-only (at least, when there
// is also a GNU_RELRO section)
// https://github.com/Gallopsled/pwntools/blob/ec9dfe108b85ac47983e9a98808fcdbb50cb0cdd/pwnlib/elf/elf.py#L1578
private static bool HasBindNowFlag(IELF elf)
{
try
{
var dynamicSection = (ELFSharp.ELF.Sections.IDynamicSection)elf.GetSection(".dynamic");
if (dynamicSection == null)
{
return false;
}

foreach (ELFSharp.ELF.Sections.DynamicEntry<ulong> entry in dynamicSection.Entries)
{
if ((entry.Tag == ELFSharp.ELF.Sections.DynamicTag.BindNow) ||
(entry.Tag == ELFSharp.ELF.Sections.DynamicTag.Flags && (entry.Value & DF_BIND_NOW) != 0) ||
(entry.Tag == ELFSharp.ELF.Sections.DynamicTag.Flags1 && (entry.Value & DF_1_NOW) != 0))
{
return true;
}
}
}
catch (InvalidCastException)
Copy link
Member

@michaelcfanning michaelcfanning Apr 15, 2021

Choose a reason for hiding this comment

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

InvalidCastException [](start = 19, length = 20)

Just curious, what's producing these exceptions? #Closed

Copy link
Member Author

@toshipiazza toshipiazza Apr 15, 2021

Choose a reason for hiding this comment

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

The project I'm working on separates executables into two files like in this post. The .dbg file contains only debug info, and therefore isn't even executable. But since it's an ELF, it gets parsed by binskim analyze. Since the file contains no executable code, the output of binskim is meaningless, so I was happy to just silence the exception.

I haven't seen any other case where this happens. #Closed

{
return false;
}

return false;
}
}
}
2 changes: 1 addition & 1 deletion src/BinSkim.Rules/RuleIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ internal static class RuleIds

// Skipping some check namespace (BA3004-3009) for future checks
public const string EnableReadOnlyRelocations = "BA3010";
public const string EnableBindNow = "BA3011";
Copy link
Member

@michaelcfanning michaelcfanning Apr 15, 2021

Choose a reason for hiding this comment

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

EnableBindNow [](start = 28, length = 13)

Good names! Let's add test cases for this rule, @eddynaka. #Closed

Copy link
Member Author

@toshipiazza toshipiazza Apr 15, 2021

Choose a reason for hiding this comment

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

FYI there are some test cases for this already:

$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.relocationsrw -Wl,-z,norelro

$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.relocationsro -Wl,-z,relro

$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.immediate_binding -Wl,-z,relro,-z,now

Spoke offline with Eddy, and it may be helpful to update the no_immediate_binding test to explicitly use RTLD_LAZY binding: -Wl,-z,lazy

$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.no_immediate_binding
#Closed


// BA3011 -- saved for a future check.
// BA3012-3029 -- saved for future non-compiler/language specific checks.
// Compiler/Language specific checks follow.
public const string UseCheckedFunctionsWithGcc = "BA3030";
Expand Down
29 changes: 28 additions & 1 deletion src/BinSkim.Rules/RuleResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/BinSkim.Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,15 @@ Modules triggering this check were:
<data name="BA3010_Pass" xml:space="preserve">
<value>The GNU_RELRO segment was present, so '{0}' is protected.</value>
</data>
<data name="BA3011_EnableBindNow_Description" xml:space="preserve">
<value>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,now' to enable this.</value>
</data>
<data name="BA3011_Error" xml:space="preserve">
<value>The BIND_NOW flag is missing from this binary, so relocation sections in '{0}' will not be marked as read only after the binary is loaded. An attacker can overwrite these to redirect control flow. Ensure you are compiling with the compiler flags '-Wl,z,now' to address this.</value>
</data>
<data name="BA3011_Pass" xml:space="preserve">
<value>The BIND_NOW flag was present, so '{0}' is protected.</value>
</data>
<data name="BA3030_Error" xml:space="preserve">
<value>No checked functions are present/used when compiling '{0}', and it was compiled with GCC--and it uses functions that can be checked. The Fortify Source flag replaces some unsafe functions with checked versions when a static length can be determined, and can be enabled by passing '-D_FORTIFY_SOURCE=2' when optimization level 2 ('-O2') is enabled. It is possible that the flag was passed, but that the compiler could not statically determine the length of any buffers/strings.</value>
</data>
Expand Down
1 change: 1 addition & 0 deletions src/BinaryParsers/VersionConstants.cs
Original file line number Diff line number Diff line change
@@ -1,5 +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
{
public static class VersionConstants
Expand Down
1 change: 1 addition & 0 deletions src/BuildSamples/ELF/build-test-binaries.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ do

# Immediate binding
$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.immediate_binding -Wl,-z,relro,-z,now
$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.lazy_binding -Wl,-z,relro,-z,lazy
$C_COMPILER empty.c -o $OUTPUT_DIR/$C_COMPILER.no_immediate_binding
done

Expand Down
3 changes: 3 additions & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
## **v2.0.0** In progress (probably May/June 2021)
* BREAKING: Change from self-contained to dotnettool.

## **v1.8.0-prerelease1** [NuGet Package](https://www.nuget.org/packages/Microsoft.CodeAnalysis.BinSkim/1.8.0-prerelease1)
* FEATURE: Add BA3011.EnableBindNow. [#363](https://github.com/microsoft/binskim/pull/363)

## **v1.7.5** [NuGet Package](https://www.nuget.org/packages/Microsoft.CodeAnalysis.BinSkim/1.7.5)
* BUGFIX: Fix import/export config using JSON file. [#349](https://github.com/microsoft/binskim/pull/349)
* FEATURE: Add compiler report rule BA4001, which is disabled by default. [#350](https://github.com/microsoft/binskim/pull/350)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,34 @@
]
},
{
"ruleId": "BA3030",
"ruleId": "BA3011",
"ruleIndex": 16,
"kind": "notApplicable",
"level": "none",
"message": {
"id": "NotApplicable_InvalidMetadata",
"arguments": [
"BinSkim.win-x64.ni.dll",
"EnableBindNow",
"image is not an ELF binary"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///Z:/src/Test.FunctionalTests.BinSkim.Driver/BaselineTestsData/BinSkim.win-x64.ni.dll",
"index": 0
}
}
}
]
},
{
"ruleId": "BA3030",
"ruleIndex": 17,
"kind": "notApplicable",
"level": "none",
"message": {
"id": "NotApplicable_InvalidMetadata",
"arguments": [
Expand All @@ -414,7 +438,7 @@
},
{
"ruleId": "BA2001",
"ruleIndex": 17,
"ruleIndex": 18,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -436,7 +460,7 @@
},
{
"ruleId": "BA2005",
"ruleIndex": 18,
"ruleIndex": 19,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -458,7 +482,7 @@
},
{
"ruleId": "BA2009",
"ruleIndex": 19,
"ruleIndex": 20,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -480,7 +504,7 @@
},
{
"ruleId": "BA2010",
"ruleIndex": 20,
"ruleIndex": 21,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -502,7 +526,7 @@
},
{
"ruleId": "BA2012",
"ruleIndex": 21,
"ruleIndex": 22,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -524,7 +548,7 @@
},
{
"ruleId": "BA2019",
"ruleIndex": 22,
"ruleIndex": 23,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -546,7 +570,7 @@
},
{
"ruleId": "BA2021",
"ruleIndex": 23,
"ruleIndex": 24,
"kind": "pass",
"level": "none",
"message": {
Expand All @@ -568,7 +592,7 @@
},
{
"ruleId": "BA2022",
"ruleIndex": 24,
"ruleIndex": 25,
"kind": "notApplicable",
"level": "none",
"message": {
Expand Down Expand Up @@ -1029,6 +1053,28 @@
},
"name": "EnableReadOnlyRelocations"
},
{
"id": "BA3011",
"fullDescription": {
"text": "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,now' to enable this."
},
"helpUri": "https://github.com/microsoft/binskim/blob/main/docs/BinSkimRules.md#rule-BA3011EnableBindNow",
"help": {
"text": "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,now' to enable this."
},
"messageStrings": {
"Pass": {
"text": "The BIND_NOW flag was present, so '{0}' is protected."
},
"Error": {
"text": "The BIND_NOW flag is missing from this binary, so relocation sections in '{0}' will not be marked as read only after the binary is loaded. An attacker can overwrite these to redirect control flow. Ensure you are compiling with the compiler flags '-Wl,z,now' to address this."
},
"NotApplicable_InvalidMetadata": {
"text": "'{0}' was not evaluated for check '{1}' as the analysis is not relevant based on observed metadata: {2}."
}
},
"name": "EnableBindNow"
},
{
"id": "BA3030",
"fullDescription": {
Expand Down