Fix usages of NamedTemporaryFiles on Windows (#486)

* Close all temporary files before handing off to external tool

* Explicitly define TemporaryFile on Windows with delete_on_close=True

* Switch ofrak.tempfile to tempfile312 and add temp_to_disk helper method

* Only use tempfile312 in ofrak_core and add it to requirements.txt

* Remove duplicate tempfile import

* Fix typing error when reopening temp file under same name

* Bump tempfile312 requirement to tempfile312~=1.0.1 which doesn't use Self

* Add changelog entry

* Use mode='wb' in temp_to_disk()

* Uncomment lines in cpio.py

* Use create_subprocess_exec with arguments instead of shell for CPIO test

disassemblers/ofrak_binary_ninja/ofrak_binary_ninja/components/binary_ninja_analyzer.py

@@ -1,5 +1,4 @@
 import logging
-import tempfile
 from dataclasses import dataclass
 from typing import Optional, List
 
@@ -29,11 +28,9 @@ async def analyze(
         self, resource: Resource, config: Optional[BinaryNinjaAnalyzerConfig] = None
     ) -> BinaryNinjaAnalysis:
         if not config:
-            resource_data = await resource.get_data()
-            temp_file = tempfile.NamedTemporaryFile()
-            temp_file.write(resource_data)
-            temp_file.flush()
-            bv = open_view(temp_file.name)
+            async with resource.temp_to_disk(delete=False) as temp_path:
+                bv = open_view(temp_path)
+
             return BinaryNinjaAnalysis(bv)
         else:
             bv = BinaryViewType.get_view_of_file(config.bndb_file)

ofrak_core/CHANGELOG.md

@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Use PyPI version of `bincopy`, upgrade to version 20.0.0 ([#528](https://github.com/redballoonsecurity/ofrak/pull/528))
 - Fix bugs on Windows arising from using `os.path` methods when only forward-slashes are acceptable ([#521](https://github.com/redballoonsecurity/ofrak/pull/521))
 - Made some changes to OFRAK test suite to improve test coverage on Windows ([#487](https://github.com/redballoonsecurity/ofrak/pull/487))
+- Fix usage of `NamedTemporaryFile` with external tools on Windows ([#486](https://github.com/redballoonsecurity/ofrak/pull/486))
 
 ### Changed
 - By default, the ofrak log is now `ofrak-YYYYMMDDhhmmss.log` rather than just `ofrak.log` and the name can be specified on the command line ([#480](https://github.com/redballoonsecurity/ofrak/pull/480))

ofrak_core/ofrak/core/apk.py

@@ -2,7 +2,7 @@
 import os
 import pathlib
 import sys
-import tempfile
+import tempfile312 as tempfile
 from subprocess import CalledProcessError
 from dataclasses import dataclass
 
@@ -101,18 +101,15 @@ async def unpack(self, resource: Resource, config=None):
         :param config:
         """
         apk = await resource.view_as(Apk)
-        data = await resource.get_data()
-        with tempfile.NamedTemporaryFile() as temp_file:
-            temp_file.write(data)
-            temp_file.flush()
+        async with resource.temp_to_disk() as temp_path:
             with tempfile.TemporaryDirectory() as temp_flush_dir:
                 cmd = [
                     "apktool",
                     "decode",
                     "--output",
                     temp_flush_dir,
                     "--force",
-                    temp_file.name,
+                    temp_path,
                 ]
                 proc = await asyncio.create_subprocess_exec(
                     *cmd,
@@ -156,7 +153,8 @@ async def pack(
         apk = await resource.view_as(Apk)
         temp_flush_dir = await apk.flush_to_disk()
         apk_suffix = ".apk"
-        with tempfile.NamedTemporaryFile(suffix=apk_suffix) as temp_apk:
+        with tempfile.NamedTemporaryFile(suffix=apk_suffix, delete_on_close=False) as temp_apk:
+            temp_apk.close()
             apk_cmd = [
                 "apktool",
                 "build",
@@ -218,13 +216,11 @@ async def identify(self, resource: Resource, config=None) -> None:
         if magic.mime == "application/vnd.android.package-archive":
             resource.add_tag(Apk)
         elif magic is not None and magic.mime in ["application/java-archive", "application/zip"]:
-            with tempfile.NamedTemporaryFile(suffix=".zip") as temp_file:
-                temp_file.write(await resource.get_data())
-                temp_file.flush()
+            async with resource.temp_to_disk(suffix=".zip") as temp_path:
                 unzip_cmd = [
                     "unzip",
                     "-l",
-                    temp_file.name,
+                    temp_path,
                 ]
                 unzip_proc = await asyncio.create_subprocess_exec(
                     *unzip_cmd,

ofrak_core/ofrak/core/binwalk.py

@@ -1,5 +1,4 @@
 import asyncio
-import tempfile
 from concurrent.futures.process import ProcessPoolExecutor
 from dataclasses import dataclass
 from typing import Dict
@@ -62,15 +61,11 @@ def __init__(
     async def analyze(self, resource: Resource, config=None) -> BinwalkAttributes:
         if not BINWALK_INSTALLED:
             raise ComponentMissingDependencyError(self, BINWALK_TOOL)
-        with tempfile.NamedTemporaryFile() as temp_file:
-            data = await resource.get_data()
-            temp_file.write(data)
-            temp_file.flush()
-
+        async with resource.temp_to_disk() as temp_path:
             # Should errors be handled the way they are in the `DataSummaryAnalyzer`? Likely to be
             # overkill here.
             offsets = await asyncio.get_running_loop().run_in_executor(
-                self.pool, _run_binwalk_on_file, temp_file.name
+                self.pool, _run_binwalk_on_file, temp_path
             )
         return BinwalkAttributes(offsets)
 

ofrak_core/ofrak/core/cpio.py

@@ -1,6 +1,6 @@
 import asyncio
 import logging
-import tempfile
+import tempfile312 as tempfile
 from dataclasses import dataclass
 from enum import Enum
 from subprocess import CalledProcessError
@@ -100,8 +100,8 @@ async def unpack(self, resource: Resource, config=None):
                 cwd=temp_flush_dir,
             )
             await proc.communicate(input=resource_data)
-            if proc.returncode:
-                raise CalledProcessError(returncode=proc.returncode, cmd=cmd)
+            # if proc.returncode:
+            #     raise CalledProcessError(returncode=proc.returncode, cmd=cmd)
             await cpio_v.initialize_from_disk(temp_flush_dir)
 
 

ofrak_core/ofrak/core/elf/lief_modifier.py

@@ -1,9 +1,9 @@
-import tempfile
 from dataclasses import dataclass
 from typing import List, Optional
 
 import lief
 
+import tempfile312 as tempfile
 from ofrak.component.modifier import Modifier
 from ofrak.model.component_model import ComponentConfig
 from ofrak.resource import Resource
@@ -66,9 +66,9 @@ async def modify(self, resource: Resource, config: LiefAddSegmentConfig) -> None
         else:
             _ = binary.add(segment)
 
-        with tempfile.NamedTemporaryFile() as temp_file:
+        with tempfile.NamedTemporaryFile(delete_on_close=False) as temp_file:
+            temp_file.close()
             binary.write(temp_file.name)
-            temp_file.flush()
             with open(temp_file.name, "rb") as f_handle:
                 new_data = f_handle.read()
         # replace all old content (old range) with new content from Lief
@@ -93,9 +93,9 @@ async def modify(self, resource: Resource, config: LiefAddSectionModifierConfig)
         section.flags = config.flags
         binary.add(section)
 
-        with tempfile.NamedTemporaryFile() as temp_file:
+        with tempfile.NamedTemporaryFile(delete_on_close=False) as temp_file:
+            temp_file.close()
             binary.write(temp_file.name)
-            temp_file.flush()
             with open(temp_file.name, "rb") as f_handle:
                 new_data = f_handle.read()
         # replace all old content (old range) with new content from Lief
@@ -117,9 +117,9 @@ async def modify(self, resource: Resource, config: LiefRemoveSectionModifierConf
             raise AttributeError(f"No section with name {config.name}")
         binary.remove(section)
 
-        with tempfile.NamedTemporaryFile() as temp_file:
+        with tempfile.NamedTemporaryFile(delete_on_close=False) as temp_file:
+            temp_file.close()
             binary.write(temp_file.name)
-            temp_file.flush()
             with open(temp_file.name, "rb") as f_handle:
                 new_data = f_handle.read()
         # replace all old content (old range) with new content from Lief

ofrak_core/ofrak/core/extfs.py

@@ -1,5 +1,5 @@
 import asyncio
-import tempfile
+import tempfile312 as tempfile
 from dataclasses import dataclass
 from subprocess import CalledProcessError
 
@@ -55,16 +55,13 @@ class ExtUnpacker(Unpacker[None]):
     external_dependencies = (_DEBUGFS,)
 
     async def unpack(self, resource: Resource, config: ComponentConfig = None) -> None:
-        with tempfile.NamedTemporaryFile(suffix=".extfs") as temp_fs_file:
-            temp_fs_file.write(await resource.get_data())
-            temp_fs_file.flush()
-
+        async with resource.temp_to_disk(suffix=".extfs") as temp_fs_path:
             with tempfile.TemporaryDirectory() as temp_dir:
                 command = [
                     "debugfs",
                     "-R",
                     f"rdump / {temp_dir}",
-                    temp_fs_file.name,
+                    temp_fs_path,
                 ]
                 proc = await asyncio.create_subprocess_exec(
                     *command,

ofrak_core/ofrak/core/filesystem.py

@@ -2,7 +2,7 @@
 import stat
 import sys
 import warnings
-import tempfile
+import tempfile312 as tempfile
 import posixpath
 from pathlib import PurePath
 from dataclasses import dataclass

ofrak_core/ofrak/core/gzip.py

@@ -3,7 +3,7 @@
 from typing import Optional
 import zlib
 from subprocess import CalledProcessError
-import tempfile
+import tempfile312 as tempfile
 
 from ofrak.component.packer import Packer
 from ofrak.component.unpacker import Unpacker
@@ -110,9 +110,9 @@ async def pack_with_zlib_module(data: bytes) -> bytes:
 
     @staticmethod
     async def pack_with_pigz(data: bytes) -> bytes:
-        with tempfile.NamedTemporaryFile() as uncompressed_file:
+        with tempfile.NamedTemporaryFile(delete_on_close=False) as uncompressed_file:
             uncompressed_file.write(data)
-            uncompressed_file.flush()
+            uncompressed_file.close()
 
             cmd = [
                 "pigz",

ofrak_core/ofrak/core/iso9660.py

@@ -1,7 +1,7 @@
 import asyncio
 import logging
+import tempfile312 as tempfile
 import posixpath
-import tempfile
 from dataclasses import dataclass
 from io import BytesIO
 from subprocess import CalledProcessError
@@ -301,7 +301,8 @@ async def pack(self, resource: Resource, config=None) -> None:
 
         iso_attrs = resource.get_attributes(ISO9660ImageAttributes)
         temp_flush_dir = await iso_view.flush_to_disk()
-        with tempfile.NamedTemporaryFile(suffix=".iso", mode="rb") as temp:
+        with tempfile.NamedTemporaryFile(suffix=".iso", mode="rb", delete_on_close=False) as temp:
+            temp.close()
             cmd = [
                 "mkisofs",
                 *(["-J"] if iso_attrs.has_joliet else []),
@@ -329,7 +330,8 @@ async def pack(self, resource: Resource, config=None) -> None:
             returncode = await proc.wait()
             if proc.returncode:
                 raise CalledProcessError(returncode=returncode, cmd=cmd)
-            new_data = temp.read()
+            with open(temp.name, "rb") as new_fh:
+                new_data = new_fh.read()
             # Passing in the original range effectively replaces the original data with the new data
             resource.queue_patch(Range(0, await resource.get_data_length()), new_data)
 

ofrak_core/ofrak/core/jffs2.py

@@ -1,6 +1,6 @@
 import asyncio
 import logging
-import tempfile
+import tempfile312 as tempfile
 from dataclasses import dataclass
 from subprocess import CalledProcessError
 
@@ -38,18 +38,14 @@ class Jffs2Unpacker(Unpacker[None]):
     external_dependencies = (JEFFERSON,)
 
     async def unpack(self, resource: Resource, config=None):
-        with tempfile.NamedTemporaryFile() as temp_file:
-            resource_data = await resource.get_data()
-            temp_file.write(resource_data)
-            temp_file.flush()
-
+        async with resource.temp_to_disk() as temp_path:
             with tempfile.TemporaryDirectory() as temp_flush_dir:
                 cmd = [
                     "jefferson",
                     "--force",
                     "--dest",
                     temp_flush_dir,
-                    temp_file.name,
+                    temp_path,
                 ]
                 proc = await asyncio.create_subprocess_exec(
                     *cmd,
@@ -73,7 +69,8 @@ class Jffs2Packer(Packer[None]):
     async def pack(self, resource: Resource, config=None):
         jffs2_view: Jffs2Filesystem = await resource.view_as(Jffs2Filesystem)
         temp_flush_dir = await jffs2_view.flush_to_disk()
-        with tempfile.NamedTemporaryFile(suffix=".sqsh", mode="rb") as temp:
+        with tempfile.NamedTemporaryFile(suffix=".sqsh", mode="rb", delete_on_close=False) as temp:
+            temp.close()
             cmd = [
                 "mkfs.jffs2",
                 "-r",
@@ -87,7 +84,8 @@ async def pack(self, resource: Resource, config=None):
             returncode = await proc.wait()
             if proc.returncode:
                 raise CalledProcessError(returncode=returncode, cmd=cmd)
-            new_data = temp.read()
+            with open(temp.name, "rb") as new_fh:
+                new_data = new_fh.read()
             # Passing in the original range effectively replaces the original data with the new data
             resource.queue_patch(Range(0, await resource.get_data_length()), new_data)
 

ofrak_core/ofrak/core/lzo.py

@@ -1,5 +1,4 @@
 import asyncio
-import tempfile
 from subprocess import CalledProcessError
 
 from ofrak.component.packer import Packer
@@ -37,27 +36,18 @@ class LzoUnpacker(Unpacker[None]):
     external_dependencies = (LZOP,)
 
     async def unpack(self, resource: Resource, config: ComponentConfig = None) -> None:
-        with tempfile.NamedTemporaryFile(suffix=".lzo") as compressed_file:
-            compressed_file.write(await resource.get_data())
-            compressed_file.flush()
-
-            cmd = [
-                "lzop",
-                "-d",
-                "-f",
-                "-c",
-                compressed_file.name,
-            ]
-            proc = await asyncio.create_subprocess_exec(
-                *cmd,
-                stdout=asyncio.subprocess.PIPE,
-                stderr=asyncio.subprocess.PIPE,
-            )
-            stdout, stderr = await proc.communicate()
-            if proc.returncode:
-                raise CalledProcessError(returncode=proc.returncode, cmd=cmd)
-
-            await resource.create_child(tags=(GenericBinary,), data=stdout)
+        cmd = ["lzop", "-d", "-f"]
+        proc = await asyncio.create_subprocess_exec(
+            *cmd,
+            stdin=asyncio.subprocess.PIPE,
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+        )
+        stdout, stderr = await proc.communicate(await resource.get_data())
+        if proc.returncode:
+            raise CalledProcessError(returncode=proc.returncode, cmd=cmd)
+
+        await resource.create_child(tags=(GenericBinary,), data=stdout)
 
 
 class LzoPacker(Packer[None]):
@@ -73,28 +63,20 @@ async def pack(self, resource: Resource, config: ComponentConfig = None):
         child_file = await lzo_view.get_child()
         uncompressed_data = await child_file.resource.get_data()
 
-        with tempfile.NamedTemporaryFile(suffix=".lzo") as uncompressed_file:
-            uncompressed_file.write(uncompressed_data)
-            uncompressed_file.flush()
-
-            cmd = [
-                "lzop",
-                "-f",
-                "-c",
-                uncompressed_file.name,
-            ]
-            proc = await asyncio.create_subprocess_exec(
-                *cmd,
-                stdout=asyncio.subprocess.PIPE,
-                stderr=asyncio.subprocess.PIPE,
-            )
-            stdout, stderr = await proc.communicate()
-            if proc.returncode:
-                raise CalledProcessError(returncode=proc.returncode, cmd=cmd)
-
-            compressed_data = stdout
-            original_size = await lzo_view.resource.get_data_length()
-            resource.queue_patch(Range(0, original_size), compressed_data)
+        cmd = ["lzop", "-f"]
+        proc = await asyncio.create_subprocess_exec(
+            *cmd,
+            stdin=asyncio.subprocess.PIPE,
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+        )
+        stdout, stderr = await proc.communicate(uncompressed_data)
+        if proc.returncode:
+            raise CalledProcessError(returncode=proc.returncode, cmd=cmd)
+
+        compressed_data = stdout
+        original_size = await lzo_view.resource.get_data_length()
+        resource.queue_patch(Range(0, original_size), compressed_data)
 
 
 MagicMimeIdentifier.register(LzoData, "application/x-lzop")