Skip to content

Fix elf stack, pe stack and pe sections memory protection #1601

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

Open
xlatbx59 wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Fix elf stack, pe stack and pe sections memory protection #1601

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e2fef9
Fix elf stack, pe stack and pe sections memory protection
xlatbx59 Nov 25, 2025
8b12ede
Fix elf stack, pe stack and pe sections memory protection according t…
xlatbx59 Nov 28, 2025
e3c8ef2
Fix pe section loading
xlatbx59 Nov 30, 2025
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
18 changes: 12 additions & 6 deletions qiling/loader/elf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ def run(self):

self.profile = self.ql.os.profile[f'OS{self.ql.arch.bits}']

# setup program stack
stack_address = self.profile.getint('stack_address')
stack_size = self.profile.getint('stack_size')
top_of_stack = stack_address + stack_size
self.ql.mem.map(stack_address, stack_size, info='[stack]')

self.path = self.ql.path

with open(self.path, 'rb') as infile:
Expand All @@ -98,6 +92,18 @@ def run(self):
elffile = ELFFile(fstream)
elftype = elffile['e_type']

stack_perm = UC_PROT_NONE
for seg in elffile.iter_segments('PT_GNU_STACK'):
stack_perm = QlLoaderELF.seg_perm_to_uc_prot(seg['p_flags'])

QlLoaderELF.seg_perm_to_uc_prot(stack_perm)
Copy link
Copy Markdown
Member

@elicn elicn Nov 28, 2025

Choose a reason for hiding this comment

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

Why iterating through segments where there is just one (or none)?
It would make more sense to get the segment by name and then extract its flags.

Also, in case segment is found, stack_perm already holds UC Protection value, not sure what happens on line 99 (converting again? where the returned value goes?)


# setup program stack
stack_address = self.profile.getint('stack_address')
stack_size = self.profile.getint('stack_size')
top_of_stack = stack_address + stack_size
self.ql.mem.map(stack_address, stack_size, stack_perm, info='[stack]')

# is it a driver?
if elftype == 'ET_REL':
self.load_driver(elffile, top_of_stack, loadbase=0x8000000)
Expand Down
39 changes: 35 additions & 4 deletions qiling/loader/pe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from unicorn import UcError
from unicorn.x86_const import UC_X86_REG_CR4, UC_X86_REG_CR8
from unicorn.unicorn_const import UC_PROT_READ, UC_PROT_WRITE, UC_PROT_EXEC, UC_PROT_NONE

from qiling.arch.x86_const import FS_SEGMENT_ADDR, GS_SEGMENT_ADDR
from qiling.const import QL_ARCH, QL_STATE
Expand Down Expand Up @@ -381,8 +382,7 @@ def load_dll(self, name: str, is_driver: bool = False) -> int:
dll_len = image_size

self.dll_size += dll_len
self.ql.mem.map(dll_base, dll_len, info=dll_name)
self.ql.mem.write(dll_base, bytes(data))
_map_sections_(self.ql, dll, dll_base, dll_len, dll_name)

if dll_base == self.dll_last_address:
self.dll_last_address = self.ql.mem.align_up(self.dll_last_address + dll_len, 0x10000)
Expand Down Expand Up @@ -813,6 +813,35 @@ def init_security_cookie(self, pe: pefile.PE, image_base: int):
cookie = secrets.randbits(self.ql.arch.bits - 16)

self.ql.mem.write_ptr(cookie_rva + image_base, cookie)

def _map_sections_(ql : Qiling, pe : pefile.PE, image_base: int, image_size : int, image_name : str):
"""Load file sections to memory, each in its own memory region protected by
its defined permissions. That allows separation of code and data, which makes
it easier to detect abnomal behavior or memory corruptions.
"""

# load the header
hdr_base = image_base
hdr_perm = UC_PROT_READ

ql.mem.map(hdr_base, image_size, hdr_perm, image_name)

# load sections
for section in pe.sections:
if not section.IMAGE_SCN_MEM_DISCARDABLE:
sec_name = section.Name.rstrip(b'\x00').decode()
sec_data = bytes(section.get_data(ignore_padding=True))
sec_base = image_base + section.VirtualAddress
sec_size = (int(section.Misc_VirtualSize / pe.OPTIONAL_HEADER.SectionAlignment) + 1) * pe.OPTIONAL_HEADER.SectionAlignment

sec_perm = sum((
section.IMAGE_SCN_MEM_READ * UC_PROT_READ,
section.IMAGE_SCN_MEM_WRITE * UC_PROT_WRITE,
section.IMAGE_SCN_MEM_EXECUTE * UC_PROT_EXEC
))

ql.mem.protect(sec_base, sec_size, sec_perm)
ql.mem.write(image_base, bytes(pe.get_memory_mapped_image()))

class QlLoaderPE(QlLoader, Process):
def __init__(self, ql: Qiling, libcache: bool):
Expand Down Expand Up @@ -890,6 +919,9 @@ def load(self, pe: Optional[pefile.PE]):
image_base = pe.OPTIONAL_HEADER.ImageBase
image_size = self.ql.mem.align_up(pe.OPTIONAL_HEADER.SizeOfImage)

if pe.OPTIONAL_HEADER.DllCharacteristics & pefile.DLL_CHARACTERISTICS['IMAGE_DLLCHARACTERISTICS_NX_COMPAT']:
self.ql.mem.protect(self.stack_address, self.stack_size, UC_PROT_WRITE | UC_PROT_READ)

# if default base address is taken, use the one specified in profile
if not self.ql.mem.is_available(image_base, image_size):
image_base = self.image_address
Expand All @@ -902,7 +934,7 @@ def load(self, pe: Optional[pefile.PE]):
self.ql.log.info(f'Loading {self.path} to {image_base:#x}')
self.ql.log.info(f'PE entry point at {self.entry_point:#x}')

self.ql.mem.map(image_base, image_size, info=f'{image_name}')
_map_sections_(self.ql, pe, image_base, image_size, image_name)
Copy link
Copy Markdown
Member

@elicn elicn Nov 28, 2025

Choose a reason for hiding this comment

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

As suggested on the UEFI PE loader, where this code was taken from, some PE files might not be suitable to be mapped like that. Setting permissions by page is doable only when the file segments are page-aligned; if they are not, one page may contain multiple segments with different permissions, we have to map the file as a whole (all RWX). We should maintain the same behavior here.

Copy link
Copy Markdown
Author

@xlatbx59 xlatbx59 Nov 29, 2025

Choose a reason for hiding this comment

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

so, should I just leave it as it is with rwx for every section?

Copy link
Copy Markdown
Member

@elicn elicn Nov 30, 2025

Choose a reason for hiding this comment

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

No. The point is that executables could be either compatible with this kind of mapping or not based on whether their sections are page-aligned [or not]. If they are not compatible, the only way is to map them as a whole with RWX permissions. Follow the UEFI PE loader to see how it works.

self.images.append(Image(image_base, image_base + pe.NT_HEADERS.OPTIONAL_HEADER.SizeOfImage, os.path.abspath(self.path)))

if self.is_driver:
Expand Down Expand Up @@ -932,7 +964,6 @@ def load(self, pe: Optional[pefile.PE]):
pe.parse_data_directories()

# done manipulating pe file; write its contents into memory
self.ql.mem.write(image_base, bytes(pe.get_memory_mapped_image()))
Comment on lines 934 to -935
Copy link
Copy Markdown
Member

@elicn elicn Nov 28, 2025

Choose a reason for hiding this comment

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

I believe file contents cannot be written till this point, mainly because of the pe.parse_data_directories 2 lines above that still manipulates the file contents.


if self.is_driver:
# security cookie can be written only after image has been loaded to memory
Expand Down