Reject colliding Decky plugin ZIP paths

docs/framey-loader.md

@@ -19,7 +19,8 @@ panel or Decky backend keeps plugin folders until each is removed separately.
 
 Frame Control's existing official-store manager downloads a published ZIP from
 Decky's content-addressed CDN, verifies its store SHA-256 hash and manifest,
-rejects unsafe ZIP members and root plugins, and swaps one managed plugin
+rejects unsafe ZIP members, path collisions, special files and root plugins,
+and swaps one managed plugin
 folder with rollback on failure. It rejects non-ARM64 ELF and Windows binaries.
 The Decky user service restarts after a file change. Framey's panel host accepts
 only API v2 bundles with `dist/index.js`; other installed plugins remain

tests/test_frame_decky_store.py

@@ -11,6 +11,7 @@ import sys
 import tempfile
 import unittest
 from unittest.mock import patch
+import warnings
 import zipfile
 
 sys.path.insert(0, str(Path(__file__).resolve().parent.parent / 'ui'))
@@ -54,6 +55,22 @@ class ArchiveTests(unittest.TestCase):
                 with self.assertRaises(ValueError):
                     frame_decky_store.archive_info(data, hashlib.sha256(data).hexdigest(), 'Example')
 
+    def test_rejects_zip_path_collisions_and_non_object_manifest(self):
+        for name in ('Example/dist/index.js', 'Example/./dist/index.js',
+                     'Example//dist/index.js'):
+            with self.subTest(name=name), warnings.catch_warnings():
+                warnings.simplefilter('ignore', UserWarning)
+                data, digest = archive(extra=(name, 'replacement'))
+                with self.assertRaisesRegex(ValueError, 'Unsafe|duplicate'):
+                    frame_decky_store.archive_info(data, digest, 'Example')
+        stream = io.BytesIO()
+        with zipfile.ZipFile(stream, 'w') as bundle:
+            bundle.writestr('Example/plugin.json', '[]')
+            bundle.writestr('Example/dist/index.js', ' ' * 1200)
+        data = stream.getvalue()
+        with self.assertRaisesRegex(ValueError, 'manifest'):
+            frame_decky_store.archive_info(data, hashlib.sha256(data).hexdigest(), 'Example')
+
 
 @unittest.skipIf(os.name == 'nt', 'remote plugin transaction runs on Linux')
 class StoreTransaction(unittest.TestCase):
@@ -146,6 +163,15 @@ class StoreTransaction(unittest.TestCase):
         self.assertEqual(installed.returncode, 0, installed.stderr)
         self.assertEqual(self.apply('remove').returncode, 0)
 
+    def test_remote_apply_rejects_colliding_zip_paths(self):
+        for name in ('Example/dist/index.js', 'Example/./dist/index.js'):
+            with self.subTest(name=name), warnings.catch_warnings():
+                warnings.simplefilter('ignore', UserWarning)
+                data, _ = archive(extra=(name, 'replacement'))
+                result = self.apply('install', data)
+                self.assertNotEqual(result.returncode, 0)
+                self.assertFalse(self.folder.exists())
+
 
 class StoreHost(unittest.TestCase):
     def test_stopped_managed_service_allows_removing_owned_plugin(self):

ui/frame_decky_store.py

@@ -77,13 +77,20 @@ def archive_info(data, expected_hash, expected_name):
         folder = manifests[0].split('/')[0]
         if not SAFE_FOLDER.fullmatch(folder):
             raise ValueError('Invalid plugin folder')
+        seen = set()
         for entry in files:
             name = entry.filename
             mode = entry.external_attr >> 16
-            if ('\\' in name or name.startswith('/') or '..' in name.split('/') or
-                name.split('/')[0] != folder or entry.file_size > 30 * 1024 * 1024 or
-                mode & 0o170000 == 0o120000):
+            kind = mode & 0o170000
+            parts = name.split('/')
+            if entry.is_dir() and parts[-1] == '':
+                parts = parts[:-1]
+            if ('\\' in name or name.startswith('/') or
+                any(part in ('', '.', '..') for part in parts) or name in seen or
+                parts[0] != folder or entry.file_size > 30 * 1024 * 1024 or
+                kind not in (0, 0o040000, 0o100000)):
                 raise ValueError('Unsafe plugin archive member')
+            seen.add(name)
             if entry.is_dir():
                 continue
             with archive.open(entry) as source:
@@ -93,6 +100,8 @@ def archive_info(data, expected_hash, expected_name):
             if header[:2] == b'MZ':
                 raise ValueError('Plugin contains a Windows binary: ' + name)
         manifest = json.loads(archive.read(manifests[0]))
+        if not isinstance(manifest, dict):
+            raise ValueError('Invalid plugin manifest')
         if manifest.get('name') != expected_name:
             raise ValueError('Plugin name differs from the official store')
         flags = manifest.get('flags', [])
@@ -142,10 +151,17 @@ try:
             files=z.infolist()
             if not files or len(files)>3000 or sum(f.file_size for f in files)>150*1024*1024:
                 raise RuntimeError('Plugin archive too large')
-            if any(f.filename.split('/')[0]!=folder or f.filename.startswith('/') or
-                   '..' in f.filename.split('/') or '\\' in f.filename or
-                   f.file_size>30*1024*1024 or f.external_attr>>16&0o170000==0o120000 for f in files):
-                raise RuntimeError('Unsafe plugin archive')
+            seen=set()
+            for f in files:
+                parts=f.filename.split('/')
+                if f.is_dir() and parts[-1]=='': parts=parts[:-1]
+                kind=f.external_attr>>16&0o170000
+                if (not parts or parts[0]!=folder or f.filename.startswith('/') or
+                    any(part in ('','.','..') for part in parts) or '\\' in f.filename or
+                    f.filename in seen or f.file_size>30*1024*1024 or
+                    kind not in (0,0o040000,0o100000)):
+                    raise RuntimeError('Unsafe plugin archive')
+                seen.add(f.filename)
             stage.mkdir(mode=0o700)
             z.extractall(stage)
             unpacked=stage/folder