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
Conversation
Toshi, thanks for this, we will review asap. #Closed |
{ | ||
// Fail | ||
context.Logger.Log(this, | ||
RuleUtilities.BuildResult(FailureLevel.Error, context, null, |
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.
FailureLevel.Error [](start = 42, length = 18)
@michaelcfanning , should we set this to note? and in the next release, we change to error based on Matt's feedback. #WontFix
} | ||
} | ||
} | ||
catch (InvalidCastException) |
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.
InvalidCastException [](start = 19, length = 20)
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
@@ -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 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
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.
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 |
Every new rule needs to either be added as 'Note' in the current release, or we need to add it to a major version revision. @eddynaka can you take care of this? Also, we need to document this great new check in our release notes. Finally, can you ensure that our comprehensive rules docs includes this check. #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.
Fixes #355