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.

Sign up for GitHub

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

Jump to bottom

BA3006.EnableNonExecutableStack #383

Merged
merged 4 commits into from
Jun 7, 2021
Merged

BA3006.EnableNonExecutableStack #383

merged 4 commits into from
Jun 7, 2021

Conversation

shaopeng-gh
Copy link
Collaborator

@shaopeng-gh shaopeng-gh commented Jun 1, 2021 •
edited by eddynaka
Loading

Refs #368

eddynaka
eddynaka reviewed Jun 3, 2021
View reviewed changes
src/BinSkim.Rules/ELFRules/BA3006.EnableNonExecutableStack.cs Outdated

if ((elfBinary.GetSegmentFlags(ELFSegmentType.PT_GNU_STACK) & SegmentFlags.Execute) != 0)
{
// The non-executable stack is not enabled from this binary,
Copy link
Contributor

@eddynaka eddynaka Jun 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

from

for this binary #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

eddynaka
eddynaka reviewed Jun 3, 2021
View reviewed changes
src/BinSkim.Rules/ELFRules/BA3006.EnableNonExecutableStack.cs Outdated
if ((elfBinary.GetSegmentFlags(ELFSegmentType.PT_GNU_STACK) & SegmentFlags.Execute) != 0)
{
// The non-executable stack is not enabled from this binary,
// so '{0}' can have vulnerability of execution of the data written on the stack.
Copy link
Contributor

@eddynaka eddynaka Jun 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

vulnerability

a vulnerability #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

eddynaka
eddynaka reviewed Jun 3, 2021
View reviewed changes
src/BinSkim.Rules/ELFRules/BA3006.EnableNonExecutableStack.cs Outdated
{
// The non-executable stack is not enabled from this binary,
// so '{0}' can have vulnerability of execution of the data written on the stack.
// Ensure you are compiling with the compiler flags '-z noexecstack' to address this.
Copy link
Contributor

@eddynaka eddynaka Jun 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

with the compiler flags

with the flag #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

eddynaka
eddynaka reviewed Jun 6, 2021
View reviewed changes
src/BinSkim.Rules/ELFRules/BA3006.EnableNonExecutableStack.cs Outdated
return;
}

// The enable non-executable stack flag was present, so '{0}' is protected.
Copy link
Contributor

@eddynaka eddynaka Jun 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The enable non-executable stack flag was present

remove the word 'enable'. #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

eddynaka
eddynaka reviewed Jun 6, 2021
View reviewed changes
src/BinaryParsers/ELFBinary/ELFBinary.cs
{
ISegment segment = ELF.Segments?.FirstOrDefault(s => (uint)s.Type == (uint)segmentType);

return segment == null ? null : (SegmentFlags?)segment.Flags;
Copy link
Contributor

@eddynaka eddynaka Jun 6, 2021 •
edited by shaopeng-gh
Loading

Choose a reason for hiding this comment

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

segment == null ? null : (SegmentFlags?)segment.Flags

try:
return (SegmentFlags?)segment.Flags ?? null; #WontFix

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the proposed code may not work since the segment can be null, if the segmentType specified can not be found,
segment.Flags will throw error

eddynaka
eddynaka approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@eddynaka eddynaka left a comment

Choose a reason for hiding this comment

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

:shipit:

@eddynaka eddynaka merged commit bff1b07 into main Jun 7, 2021
@eddynaka eddynaka deleted the users/shaopeng-gh/nonexecutablestack branch June 7, 2021 21:12
@eddynaka eddynaka linked an issue Jun 11, 2021 that may be closed by this pull request
[RULE REQUEST] Enable Non-Executable Stack #368
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddynaka eddynaka eddynaka approved these changes

@michaelcfanning michaelcfanning Awaiting requested review from michaelcfanning

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RULE REQUEST] Enable Non-Executable Stack
2 participants
@shaopeng-gh @eddynaka