extremecoders-re / pyinstxtractor

PyInstaller Extractor
GNU General Public License v3.0
2.93k stars 612 forks source link

Absolute path leads to PermissionError during extraction #67

Closed mssalvatore closed 1 year ago

mssalvatore commented 1 year ago

Problem

When attempting to extract certain PyInstaller binaries, I get the following error:

$ python pyinstxtractor.py monkey-linux-64-pyinstaller-5.11.0
[+] Pyinstaller version: 2.1+
[+] Python version: 3.11
[+] Length of package: 21197853 bytes
[+] Found 234 files in CArchive
[+] Beginning extraction...please standby
[+] Possible entry point: pyiboot01_bootstrap.pyc
[+] Possible entry point: pyi_rth_inspect.pyc
[+] Possible entry point: pyi_rth_pkgutil.pyc
[+] Possible entry point: pyi_rth_setuptools.pyc
[+] Possible entry point: pyi_rth_pkgres.pyc
[+] Possible entry point: pyi_rth_multiprocessing.pyc
[+] Possible entry point: main.pyc
Traceback (most recent call last):
  File "/home/msalvatore/compost/extract/pyinstxtractor.py", line 454, in <module>
    main()
  File "/home/msalvatore/compost/extract/pyinstxtractor.py", line 443, in main
    arch.extractFiles()
  File "/home/msalvatore/compost/extract/pyinstxtractor.py", line 292, in extractFiles
    os.makedirs(basePath)
  File "<frozen os>", line 225, in makedirs
PermissionError: [Errno 13] Permission denied: '/common'

I've tried with a similar binary built with PyInstaller 4.9 and I get roughly the same message. It appears that if certain contents have an absolute path, an attempt is made to extract them outside of the extraction directory.

Security

If a pyinstxtractor user were tricked into extracting a specially crafted executable, an attacker could write arbitrary files to the user's file system. This is similar to a ZipSlip vulnerability.

Severity: Low

For some guidance and caveats on how to fix directory traversal vulnerabilities in Python code, see this blog post.

Binaries for testing

I've attached two zip archives. Each contains a single binary: one built with PyInstaller 5.11.0 and the other built with PyInstaller 4.9 (GitHub has file size and type limits that forced me to upload separate zip archives). Both raise this error when extracted with the version of pyinstxtractor from commit cad8c74542fc33605d.

monkey-linux-64-pyinstaller-4.9.zip monkey-linux-64-pyinstaller-5.11.0.zip

Workaround

I hacked together the changes below to work around this issue. This doesn't necessarily address the security issues, but it does make pyinstxtractor work for my purposes.

diff --git a/pyinstxtractor.py b/pyinstxtractor.py
index 88c0097..529780c 100644
--- a/pyinstxtractor.py
+++ b/pyinstxtractor.py
@@ -89,6 +89,7 @@ import struct
 import marshal
 import zlib
 import sys
+from pathlib import Path
 from uuid import uuid4 as uniquename

@@ -285,7 +286,7 @@ class PyInstArchive:
                 # These are runtime options, not files
                 continue

-            basePath = os.path.dirname(entry.name)
+            basePath = Path(extractionDir) / os.path.dirname(entry.name).lstrip("/")
             if basePath != '':
                 # Check if path exists, create if not
                 if not os.path.exists(basePath):
@@ -312,7 +313,7 @@ class PyInstArchive:
                     # < pyinstaller 5.3
                     if self.pycMagic == b'\0' * 4: 
                         self.pycMagic = data[0:4]
-                    self._writeRawData(entry.name + '.pyc', data)
+                    self._writeRawData(str(Path(extractionDir) / (entry.name + '.pyc')), data)

                 else:
                     # >= pyinstaller 5.3
@@ -323,7 +324,7 @@ class PyInstArchive:
                     self._writePyc(entry.name + '.pyc', data)

             else:
-                self._writeRawData(entry.name, data)
+                self._writeRawData(str(Path(extractionDir) / entry.name.lstrip("/")), data)

                 if entry.typeCmprsData == b'z' or entry.typeCmprsData == b'Z':
                     self._extractPyz(entry.name)
extremecoders-re commented 1 year ago

Thanks for the detailed report! Much appreciated.