-
Notifications
You must be signed in to change notification settings - Fork 155
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
Changes from 2 commits
e7fd4f4
aaefdb0
518050d
31abbdd
7675329
12adff9
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 |
---|---|---|
@@ -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 EnableBineNow : ELFBinarySkimmerBase | ||
{ | ||
private const uint DF_BIND_NOW = 0x8; | ||
private const uint DF_1_NOW = 0x1; | ||
|
||
/// <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; | ||
} | ||
|
||
// 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 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) | ||
{ | ||
return false; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
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, | ||
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.
@michaelcfanning , should we set this to note? and in the next release, we change to error based on Matt's feedback. #WontFix |
||
nameof(RuleResources.BA3011_Error), | ||
context.TargetUri.GetFileName())); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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"; | ||||||||||
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.
Good names! Let's add test cases for this rule, @eddynaka. #Closed 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. FYI there are some test cases for this already:
Spoke offline with Eddy, and it may be helpful to update the no_immediate_binding test to explicitly use RTLD_LAZY binding:
|
||||||||||
|
||||||||||
// 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"; | ||||||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
// 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 | ||
{ | ||
public const string Prerelease = "-prerelease1"; | ||
public const string AssemblyVersion = "1.7.5" + ".0"; | ||
public const string FileVersion = "1.7.5" + ".0"; | ||
public const string Version = AssemblyVersion + Prerelease; | ||
} | ||
} | ||
// 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 | ||
{ | ||
public const string Prerelease = "-prerelease1"; | ||
public const string AssemblyVersion = "1.7.5" + ".0"; | ||
public const string FileVersion = "1.7.5" + ".0"; | ||
public const string Version = AssemblyVersion + Prerelease; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, what's producing these exceptions? #Closed
There was a problem hiding this comment.
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