redballoonsecurity / ofrak

OFRAK: unpack, modify, and repack binaries.
https://ofrak.com
Other
1.86k stars 127 forks source link

FreespaceAnalyzer incorrectly caches free region information #317

Open rbs-alexr opened 1 year ago

rbs-alexr commented 1 year ago

What is the problem? (Here is where you provide a complete Traceback.) When I run the FreespaceAnalyzer on an Elf first (by viewing it as Allocatable), it correctly reports no free space. If I then create free space and then run the FreespaceAnalyzer again, it still reports having no free space. This is because the results are cached and have not been updated

Please provide some information about your environment. At minimum we would like the following information on your platform and Python environment:

python3 -m pip freeze
whoops

If you've discovered it, what is the root cause of the problem? The results of the FreespaceAnalyzer are cached and are not invalidated by the PartialFreespaceModifier, so if the FreespaceAnalyzer is run again it will not have the correct information

How often does the issue happen? Whenever you use the FreespaceAnalyzer

What are the steps to reproduce the issue? Adding the following test to test_free_space.py will fail as a result of the erroneous behavior described above:

async def test_multiple_free_space_mods(resource_under_test: Resource):
    resource_under_test.add_tag(Allocatable)
    await resource_under_test.save()
    allocatable = await resource_under_test.view_as(Allocatable)
    assert len(allocatable.free_space_ranges) == 0
    data_length = await resource_under_test.get_data_length()
    config = PartialFreeSpaceModifierConfig(
        MemoryPermissions.RX,
        range_to_remove=Range.from_size(0, data_length - 10),
        fill=b"\xfe\xed\xfa\xce",
    )
    await resource_under_test.run(PartialFreeSpaceModifier, config)
    free_space = await resource_under_test.get_only_child_as_view(FreeSpace)
    free_space_data = await free_space.resource.get_data()
    assert free_space_data == config.fill + (b"\x00" * (data_length - 10 - len(config.fill)))
    allocatable = await resource_under_test.view_as(Allocatable)
    assert allocatable.free_space_ranges[MemoryPermissions.RX] == [Range(0x0, 0xf6)]

It fails with a KeyError for the last assert because free_space_ranges is {}

How would you implement this fix? Honestly not totally sure, but the FreeSpaceAnalyzer should be re-run every time the resource is viewed as Allocatable rather than use the cached values. Maybe this involves removing the Allocatable tag from the resource at the end of the FreeSpaceAnalyzer so that the Analyzer is re-run.

Are there any (reasonable) alternative approaches? Not that I am aware of

Are you interested in implementing it yourself? I don't think I would be very efficient/helpful fixing this given my limited knowledge of the internals of OFRAK.

whyitfor commented 1 year ago

One possible stopgap mitigation would be to have free space modifiers invalidate Allocatables if they exist. This doesn't seem to work. I tried adding this to the end of the PartialFreeSpaceModifier:

        child = await mem_region_view.resource.create_child_from_view(
            FreeSpace(
                freed_range.start,
                freed_range.length(),
                config.permissions,
            ),
            data_range=patch_range,
        )
        try:
            allocatable = await child.get_only_ancestor_as_view(
                Allocatable, r_filter=ResourceFilter.with_tags(Allocatable)
            )
            allocatable.resource.remove_attributes(AttributesType[Allocatable])
        except NotFoundError:
            pass

It results in an error:

    @classmethod
    def create(cls: Type[RV], resource_model: ResourceModel) -> RV:
        attributes_fields_values: Dict[str, Any] = dict()
        for required_attrs_type in cls.composed_attributes_types:
            if required_attrs_type not in resource_model.attributes:
>               raise ValueError(
                    f"Required attributes type {required_attrs_type.__name__} not "
                    f"provided in {resource_model.attributes}"
                )
E               ValueError: Required attributes type AttributesType[Allocatable] not provided in {<class 'ofrak.model._auto_attributes.AttributesType[Addressable]'>: AttributesType[Addressable](virtual_address=0), <class 'ofrak.model._auto_attributes.AttributesType[MemoryRegion]'>: AttributesType[MemoryRegion](size=256), <class 'ofrak.model.resource_model.Data'>: Data(_offset=0, _length=256)}
whyitfor commented 1 year ago

Appending the following to the end of the PartialFreeSpaceModifier does seem to fix the test:

        try:
            allocatable = await child.get_only_ancestor_as_view(
                Allocatable, r_filter=ResourceFilter.with_tags(Allocatable)
            )
            perm_list = allocatable.free_space_ranges.get(config.permissions)
            if perm_list is None:
                perm_list = list()
                allocatable.free_space_ranges[config.permissions] = perm_list
            perm_list.append(freed_range)
        except NotFoundError:
            pass

This might be a way forward. Should Allocatable have an update method to add new ranges to it (and perform this saving)?