Skip to content

Fix undeclared next_section_virtual_address usage #429

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

Merged
nightlark merged 2 commits into from
Apr 1, 2026
+18 −14
Merged

Fix undeclared next_section_virtual_address usage #429

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9366cec
Fix undeclared next_section_virtual_address usage
gdesmar Sep 23, 2024
fdfe543
Save the original section index and use it for logging.
gdesmar Mar 30, 2026
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
32 changes: 18 additions & 14 deletions pefile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,7 @@ def __init__(self, *args, **kwargs):
self.VirtualAddress_adj = None
self.section_min_addr = None
self.section_max_addr = None
self.index_in_file = None

def get_PointerToRawData_adj(self):
if self.PointerToRawData_adj is None and self.PointerToRawData is not None:
Expand Down Expand Up @@ -3679,20 +3680,8 @@ def parse_sections(self, offset, max_offset=0x10000000):
# Set the section's flags according to the Characteristics member
set_flags(section, section.Characteristics, section_flags)

if section.__dict__.get(
"IMAGE_SCN_MEM_WRITE", False
) and section.__dict__.get("IMAGE_SCN_MEM_EXECUTE", False):
if section.Name.rstrip(b"\x00") == b"PAGE" and self.is_driver():
# Drivers can have a PAGE section with those flags set without
# implying that it is malicious
pass
else:
self.__warnings.append(
f"Suspicious flags set for section {i}. "
"Both IMAGE_SCN_MEM_WRITE and IMAGE_SCN_MEM_EXECUTE are set. "
"This might indicate a packed executable."
)

# Set the section's original index as it may change after being sorted by VirtualAddress
Copy link
Copy Markdown
Collaborator

@j-t-1 j-t-1 Mar 30, 2026

Choose a reason for hiding this comment

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

From the Microsoft official documentation:

The number of entries in the section table is given by the NumberOfSections field in the file header. Entries in the section table are numbered starting from one (1).

Due to us not doing, this is fine for this PR. After this PR could look into whether we need to change to one indexing or just add a comment to explain we use zero indexing. Check what PE-bear or similar does?

Copy link
Copy Markdown
Collaborator

@nightlark nightlark Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

A comment saying we use zero indexing would be good. The output has been zero indexed for long enough that I don't think we want to change it at this time (it would also be a change that would be good to get thoughts from @erocarrera)

section.index_in_file = i
self.sections.append(section)

# Sort the sections by their VirtualAddress and add a field to each of them
Expand All @@ -3707,6 +3696,21 @@ def parse_sections(self, offset, max_offset=0x10000000):
idx + 1
].VirtualAddress

for section in self.sections:
Copy link
Copy Markdown
Collaborator

@j-t-1 j-t-1 Mar 30, 2026

Choose a reason for hiding this comment

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

We may be able to use getattr(object, name[, default]) here and throughout the file?

getattr(section, "IMAGE_SCN_MEM_WRITE", False) and getattr(section, "IMAGE_SCN_MEM_EXECUTE", False)

Better outside of this PR as is not just in this function.

if section.__dict__.get(
"IMAGE_SCN_MEM_WRITE", False
) and section.__dict__.get("IMAGE_SCN_MEM_EXECUTE", False):
if section.Name.rstrip(b"\x00") == b"PAGE" and self.is_driver():
# Drivers can have a PAGE section with those flags set without
# implying that it is malicious
pass
else:
self.__warnings.append(
f"Suspicious flags set for section {section.index_in_file}. "
"Both IMAGE_SCN_MEM_WRITE and IMAGE_SCN_MEM_EXECUTE are set. "
"This might indicate a packed executable."
)

if self.FILE_HEADER.NumberOfSections > 0 and self.sections:
return (
offset + self.sections[0].sizeof() * self.FILE_HEADER.NumberOfSections
Expand Down
Loading