Improved allocation of .bss(-like) sections

ofrak_core/CHANGELOG.md

@@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Add generic DecompilationAnalysis classes. ([#453](https://github.com/redballoonsecurity/ofrak/pull/453))
 - `PatchFromSourceModifier` bundles src and header files into same temporary directory with BOM and FEM ([#517](https://github.com/redballoonsecurity/ofrak/pull/517))
 - Add support for running on Windows to the `Filesystem` component. ([#521](https://github.com/redballoonsecurity/ofrak/pull/521))
+- Add new method for allocating `.bss` sections using free space ranges that aren't mapped to data ranges. ([#505](https://github.com/redballoonsecurity/ofrak/pull/505))
 
 ### Fixed
 - Improved flushing of filesystem entries (including symbolic links and other types) to disk. ([#373](https://github.com/redballoonsecurity/ofrak/pull/373))

ofrak_core/ofrak/core/patch_maker/modifiers.py

@@ -21,6 +21,7 @@
 from ofrak.resource import Resource
 from ofrak.service.resource_service_i import ResourceFilter, ResourceSort, ResourceSortDirection
 from ofrak_type.memory_permissions import MemoryPermissions
+from ofrak_type.error import NotFoundError
 
 LOGGER = logging.getLogger(__file__)
 
@@ -189,7 +190,15 @@ def from_fem(fem: FEM) -> "SegmentInjectorModifierConfig":
         for segment in fem.executable.segments:
             if segment.length == 0:
                 continue
-            segment_data = exe_data[segment.offset : segment.offset + segment.length]
+
+            if segment.is_bss:
+                # It's possible for NOBITS sections like .bss to be allocated into RW MemoryRegions
+                # that are mapped to data. In that case, we should zero out the region instead
+                # of patching the arbitrary data in the FEM
+                segment_data = b"\0" * segment.length
+            else:
+                segment_data = exe_data[segment.offset : segment.offset + segment.length]
+
             extracted_segments.append((segment, segment_data))
         return SegmentInjectorModifierConfig(tuple(extracted_segments))
 
@@ -220,15 +229,14 @@ async def modify(self, resource: Resource, config: SegmentInjectorModifierConfig
         injection_tasks: List[Tuple[Resource, BinaryInjectorModifierConfig]] = []
 
         for segment, segment_data in config.segments_and_data:
-            if segment.length == 0 or segment.vm_address == 0:
+            if segment.length == 0 or not segment.is_allocated:
                 continue
             if segment.length > 0:
                 LOGGER.debug(
                     f"    Segment {segment.segment_name} - {segment.length} "
                     f"bytes @ {hex(segment.vm_address)}",
                 )
-            if segment.segment_name.startswith(".bss"):
-                continue
+
             if segment.segment_name.startswith(".rela"):
                 continue
             if segment.segment_name.startswith(".got"):
@@ -238,18 +246,31 @@ async def modify(self, resource: Resource, config: SegmentInjectorModifierConfig
                 # See PatchFromSourceModifier
                 continue
 
-            patches = [(segment.vm_address, segment_data)]
-            region = MemoryRegion.get_mem_region_with_vaddr_from_sorted(
-                segment.vm_address, sorted_regions
-            )
-            if region is None:
+            try:
+                region = MemoryRegion.get_mem_region_with_vaddr_from_sorted(
+                    segment.vm_address, sorted_regions
+                )
+            except NotFoundError:
+                # uninitialized section like .bss mapped to arbitrary memory range without corresponding
+                # MemoryRegion resource, no patch needed.
+                if segment.is_bss:
+                    continue
+                raise
+
+            region_mapped_to_data = region.resource.get_data_id() is not None
+            if region_mapped_to_data:
+                patches = [(segment.vm_address, segment_data)]
+                injection_tasks.append((region.resource, BinaryInjectorModifierConfig(patches)))
+            else:
+                if segment.is_bss:
+                    # uninitialized section like .bss mapped to arbitrary memory range without corresponding
+                    # data on a resource, no patch needed.
+                    continue
                 raise ValueError(
                     f"Cannot inject patch because the memory region at vaddr "
-                    f"{hex(segment.vm_address)} is None"
+                    f"{hex(segment.vm_address)} is not mapped to data"
                 )
 
-            injection_tasks.append((region.resource, BinaryInjectorModifierConfig(patches)))
-
         for injected_resource, injection_config in injection_tasks:
             result = await injected_resource.run(BinaryInjectorModifier, injection_config)
             # The above can patch data of any of injected_resources' descendants or ancestors

ofrak_core/test_ofrak/components/free_space_components_test/mock_tree_struct.py

@@ -3,7 +3,7 @@
 from ofrak import OFRAKContext
 from ofrak.core.memory_region import MemoryRegion
 from ofrak.resource import Resource
-from ofrak.core.free_space import Allocatable
+from ofrak.core.free_space import Allocatable, RuntimeFreeSpace
 
 FreeSpaceTreeType = Tuple[MemoryRegion, Optional[List["FreeSpaceTreeType"]]]
 
@@ -28,7 +28,10 @@ async def inflate_tree(tree: FreeSpaceTreeType, ofrak_context: OFRAKContext) ->
 
 async def _inflate_node(parent: MemoryRegion, node: FreeSpaceTreeType):
     raw_node_region, children = node
-    node_r = await parent.create_child_region(raw_node_region)
+    if isinstance(raw_node_region, RuntimeFreeSpace):
+        node_r = await parent.resource.create_child_from_view(raw_node_region, data_range=None)
+    else:
+        node_r = await parent.create_child_region(raw_node_region)
     node_r.add_view(raw_node_region)
     await node_r.save()
     if children:

ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_allocate.py

@@ -20,6 +20,7 @@
 )
 from ofrak_patch_maker.patch_maker import PatchMaker
 from ofrak_patch_maker.toolchain.model import (
+    Segment,
     ToolchainConfig,
     BinFileType,
     CompilerOptimizationLevel,
@@ -58,15 +59,24 @@ def ofrak(ofrak):
 @pytest.fixture
 def mock_allocatable():
     return Allocatable(
-        {
+        free_space_ranges={
             MemoryPermissions.RX: [
                 Range(0x100, 0x110),
                 Range(0x80, 0xA0),
                 Range(0xC0, 0xE0),
                 Range(0x0, 0x40),
                 Range(0x120, 0x200),
+            ],
+            MemoryPermissions.RW: [
+                Range(0x400, 0x410),
+            ],
+        },
+        dataless_free_space_ranges={
+            MemoryPermissions.RW: [
+                Range(0xD000, 0xD020),
+                Range(0xD130, 0xD200),
             ]
-        }
+        },
     )
 
 
@@ -79,6 +89,7 @@ class AllocateTestCase:
     alignment: Optional[int] = 4
     within_range: Optional[Range] = None
     mem_permissions: MemoryPermissions = MemoryPermissions.RX
+    with_data: bool = True
 
 
 ALLOCATE_TEST_CASES = [
@@ -147,7 +158,28 @@ class AllocateTestCase:
         "allocate with memory permissions not present",
         0x100,
         None,
-        mem_permissions=MemoryPermissions.W,
+        mem_permissions=MemoryPermissions.RWX,
+    ),
+    AllocateTestCase(
+        "successful non-fragmented with_data=False allocation prefers dataless range",
+        0x20,
+        [
+            Range(0xD000, 0xD020),
+        ],
+        mem_permissions=MemoryPermissions.RW,
+        with_data=False,
+    ),
+    AllocateTestCase(
+        "successful fragmented with_data=False allocation falls back to data mapped range",
+        0x100,
+        [
+            Range(0x400, 0x410),
+            Range(0xD000, 0xD020),
+            Range(0xD130, 0xD200),
+        ],
+        mem_permissions=MemoryPermissions.RW,
+        min_fragment_size=0x10,
+        with_data=False,
     ),
 ]
 
@@ -166,6 +198,7 @@ async def test_allocate(ofrak_context: OFRAKContext, test_case: AllocateTestCase
             test_case.alignment,
             test_case.min_fragment_size,
             test_case.within_range,
+            test_case.with_data,
         )
         assert all([r in test_case.expected_allocation for r in alloc])
     else:
@@ -176,6 +209,7 @@ async def test_allocate(ofrak_context: OFRAKContext, test_case: AllocateTestCase
                 test_case.alignment,
                 test_case.min_fragment_size,
                 test_case.within_range,
+                test_case.with_data,
             )
 
 
@@ -185,7 +219,8 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir):
         f.write(
             inspect.cleandoc(
                 """
-            static int global_arr[256] = {0};
+            static int global_arr[64] __attribute__((section(".bss.new"))) = {0};
+            static int global_arr_legacy[256] __attribute__((section(".bss.legacy"))) = {0};
 
             int main_supplement(int a, int b)
             {
@@ -208,7 +243,7 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir):
                int c = -38;
                int d = main_supplement(a, b) * c;
                (void) d;
-               return foo(global_arr);
+               return foo(global_arr_legacy);
             }
 
             """
@@ -230,7 +265,7 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir):
         no_jump_tables=True,
         no_bss_section=False,
         create_map_files=True,
-        compiler_optimization_level=CompilerOptimizationLevel.FULL,
+        compiler_optimization_level=CompilerOptimizationLevel.NONE,
         debug_info=True,
     )
 
@@ -253,25 +288,29 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir):
     resource = await ofrak_context.create_root_resource("test_allocate_bom", b"\x00")
     resource.add_view(
         Allocatable(
-            {
+            free_space_ranges={
                 MemoryPermissions.RX: [
                     Range(0x100, 0x110),
                     Range(0x80, 0xA0),
                     Range(0xC0, 0xE0),
                     Range(0x0, 0x40),
                     Range(0x120, 0x200),
                 ]
-            }
+            },
+            dataless_free_space_ranges={MemoryPermissions.RW: [Range(0xD000, 0xD100)]},
         )
     )
     await resource.save()
 
     allocatable = await resource.view_as(Allocatable)
 
-    patch_config = await allocatable.allocate_bom(bom)
+    with pytest.warns(DeprecationWarning):
+        patch_config = await allocatable.allocate_bom(bom)
 
     assert len(patch_config.segments) == 1
-    for segments in patch_config.segments.values():
-        seg = segments[0]
-        assert seg.segment_name == ".text"
-        assert seg.vm_address == 0x120
+    (segments,) = patch_config.segments.values()
+    segments_by_name = {seg.segment_name: seg for seg in segments}
+
+    assert segments_by_name[".text"].vm_address == 0x120
+    assert segments_by_name[".bss.new"].vm_address == 0xD000
+    assert segments_by_name[".bss.legacy"].vm_address == Segment.BSS_LEGACY_VADDR