Fix the cross-provider review's findings

- APK reader: cap AndroidManifest.xml, resources.arsc and icon sizes before
  inflating them (APKs can come from install links), and follow resource
  references without cycles and with a result budget.
- Sideloading: reserve devkit-steam (SteamOS's sideloaded-client trampoline);
  the install dialog warns when a name replaces an installed title; a
  manifest's exe may name the program as it is in the archive, above the
  folder the installer steps into.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/test_frame_apk.py

@@ -134,6 +134,26 @@ class ApkInfo(unittest.TestCase):
             with self.assertRaises(frame_apk.ApkError):
                 self.read(data)
 
+    def test_refuses_oversized_members(self):
+        # An APK from a website mustn't make the server inflate gigabytes.
+        data = apk({'AndroidManifest.xml': manifest('com.example.big', 0x7f010000, 0x7f010001, 21)})
+        limit, frame_apk.MAX_MANIFEST = frame_apk.MAX_MANIFEST, 16
+        try:
+            with self.assertRaises(frame_apk.ApkError):
+                self.read(data)
+        finally:
+            frame_apk.MAX_MANIFEST = limit
+
+    def test_reference_cycles_and_fan_out_are_bounded(self):
+        res = frame_apk.Resources(b'')
+        ref = frame_apk.T_REF
+        res.entries = {1: [('', 0, ref, 1)] * 5}  # five references to itself
+        self.assertEqual(res.values(1), [])
+        # Five references at each of five hops: 3125 leaves without a budget.
+        res.entries = {i: [('', 0, ref, i + 1)] * 5 for i in range(1, 6)}
+        res.entries[6] = [('', 0, frame_apk.T_STRING, 0)]
+        self.assertEqual(len(res.values(1)), frame_apk.MAX_VALUES)
+
 
 if __name__ == '__main__':
     unittest.main()

tests/test_frame_titles.py

@@ -142,6 +142,17 @@ class Targets(unittest.TestCase):
         with self.assertRaises(FrameError):
             frame_titles._choose(p, '../outside.exe')
 
+    def test_exe_path_from_above_the_unwrapped_folder(self):
+        # A manifest names the program as it is in the archive: Game/B.exe, not B.exe.
+        p = self.plan({'Game/A.exe': pe(0x8664), 'Game/B.exe': pe(0x8664)}, 'Game')
+        self.assertEqual(p['unwrapped'], 'Game')
+        frame_titles._choose(p, 'Game/B.exe')
+        self.assertEqual(p['target'], 'B.exe')
+        frame_titles._choose(p, 'Game\\A.exe')
+        self.assertEqual(p['target'], 'A.exe')
+        with self.assertRaises(FrameError):
+            frame_titles._choose(p, 'Game/../../outside.exe')
+
 
 class Zips(unittest.TestCase):
     def setUp(self):
@@ -317,6 +328,8 @@ class Names(unittest.TestCase):
     def test_title_id(self):
         self.assertEqual(frame_titles.title_id('Hollow Knight: Silksong!'), 'Hollow_Knight_Silksong')
         self.assertEqual(frame_titles.title_id('steam'), 'steam-game')     # Valve's reserved sideload names
+        self.assertEqual(frame_titles.title_id('Devkit Steam'), 'Devkit_Steam')
+        self.assertEqual(frame_titles.title_id('devkit-steam'), 'devkit-steam-game')  # the trampoline file
         self.assertEqual(frame_titles.title_id('--rm -rf /'), 'rm_-rf')
         self.assertEqual(len(frame_titles.title_id('x' * 200)), 64)
         with self.assertRaises(FrameError):

ui/frame_apk.py

@@ -12,6 +12,12 @@ import zipfile
 ATTR = {0x01010001: 'label', 0x01010002: 'icon', 0x01010003: 'name',
         0x0101021b: 'versionCode', 0x0101021c: 'versionName', 0x0101020c: 'minSdkVersion'}
 T_REF, T_STRING, T_INT_DEC, T_INT_HEX = 0x01, 0x03, 0x10, 0x11
+# APKs can come from websites (install links), so nothing read from one may be
+# unbounded. zipfile stops at a member's declared size, so checking it is enough.
+MAX_MANIFEST = 16 * 1024**2
+MAX_ARSC = 128 * 1024**2      # real ones are a few MB; the largest apps' tens of MB
+MAX_ICON = 8 * 1024**2
+MAX_VALUES = 256              # resolved values per reference, across all its hops
 
 
 class ApkError(Exception):
@@ -129,15 +135,21 @@ class Resources:
                 resid = (pid << 24) | (tid << 16) | index
                 self.entries.setdefault(resid, []).append((language, density, dtype, value))
 
-    def values(self, resid, depth=0):
-        """[(language, density, type, data)] with references followed."""
+    def values(self, resid, depth=0, seen=frozenset()):
+        """[(language, density, type, data)] with references followed, at most
+        MAX_VALUES of them and never round a cycle."""
         out = []
+        seen = seen | {resid}
         for lang, dens, dtype, value in self.entries.get(resid, []):
+            if len(out) >= MAX_VALUES:
+                break
             if dtype == T_REF and depth < 5:
-                out += [(lang or l2, dens or d2, t2, v2) for l2, d2, t2, v2 in self.values(value, depth + 1)]
+                if value not in seen:
+                    out += [(lang or l2, dens or d2, t2, v2)
+                            for l2, d2, t2, v2 in self.values(value, depth + 1, seen)]
             else:
                 out.append((lang, dens, dtype, value))
-        return out
+        return out[:MAX_VALUES]
 
     def string(self, dtype, value):
         return self.strings[value] if dtype == T_STRING and value < len(self.strings) else None
@@ -171,6 +183,13 @@ def _icons(attr, res):
     return [s for _, s in sorted(vals, key=lambda x: -x[0]) if s]
 
 
+def _read(z, name, limit):
+    size = z.getinfo(name).file_size
+    if size > limit:
+        raise ApkError(f'{name} in the APK is {size / 1024**2:.0f} MB, more than a real one ({limit // 1024**2} MB)')
+    return z.read(name)
+
+
 def apk_info(path):
     """Package, label, version, min_sdk, abis and the best PNG icon inside the APK."""
     try:
@@ -182,8 +201,8 @@ def apk_info(path):
         if 'AndroidManifest.xml' not in names:
             raise ApkError('not an APK: no AndroidManifest.xml')
         try:
-            elements = manifest_elements(z.read('AndroidManifest.xml'))
-            res = Resources(z.read('resources.arsc') if 'resources.arsc' in names else b'')
+            elements = manifest_elements(_read(z, 'AndroidManifest.xml', MAX_MANIFEST))
+            res = Resources(_read(z, 'resources.arsc', MAX_ARSC) if 'resources.arsc' in names else b'')
         except (struct.error, IndexError, zipfile.BadZipFile) as e:
             raise ApkError(f'could not read the APK manifest: {e}')
         tags = {}
@@ -212,11 +231,11 @@ def apk_info(path):
 def _icon_png(z, names, icons):
     for icon in icons:
         if icon.endswith('.png') and icon in names:
-            return z.read(icon)
+            return _read(z, icon, MAX_ICON)
     # Adaptive icons are XML; fall back to the largest launcher PNG.
     pngs = sorted((n for n in names if n.endswith('.png') and 'ic_launcher' in n and 'foreground' not in n),
                   key=lambda n: z.getinfo(n).file_size)
-    return z.read(pngs[-1]) if pngs else None
+    return _read(z, pngs[-1], MAX_ICON) if pngs else None
 
 
 if __name__ == '__main__':

ui/frame_titles.py

@@ -32,7 +32,9 @@ STAMP = '.frame-control-stamp'
 PY = 'python3 ~/' + UTILS + '/'
 
 # Valve's reserved sideload names: uploading one of these replaces the Steam client itself.
-RESERVED_IDS = ('steam', 'steamdeckard', 'steamvr', 'steamvrdeckard')
+# devkit-steam is the trampoline file that switches SteamOS to a sideloaded client
+# (select_steam.sh); a folder there breaks Valve's devkit tools.
+RESERVED_IDS = ('steam', 'steamdeckard', 'steamvr', 'steamvrdeckard', 'devkit-steam')
 ID_RE = re.compile(r'^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$')
 DIR_RE = re.compile(r'^/[A-Za-z0-9_./-]+$')
 
@@ -422,6 +424,7 @@ def inspect(path, name=None):
     work = None
     try:
         if os.path.isdir(path):
+            base = path
             root = _unwrap(path)
             if _has_links(root):
                 # scp -r follows links, so a link out of the folder could upload
@@ -431,20 +434,24 @@ def inspect(path, name=None):
         elif path.lower().endswith('.zip'):
             work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
             extract_zip(path, work)
+            base = work
             root = _unwrap(work)
         elif classify(path):
             # A single executable is uploaded on its own; don't copy a whole Downloads folder.
             work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
             shutil.copy2(path, os.path.join(work, os.path.basename(path)))
-            root = work
+            base = root = work
         else:
             raise FrameError(f'{os.path.basename(path)} is not a .zip, a folder or a program')
         title = name or display_name(os.path.basename(path.rstrip('/\\')))
         found = candidates(root, title)
         if not found:
             raise FrameError(f'no Linux or Windows program found in {os.path.basename(path)}')
+        # An exe path may be given as it is in the archive, above the folder _unwrap stepped into.
+        unwrapped = os.path.relpath(root, os.path.realpath(base)).replace(os.sep, '/')
         plan = {'source': os.path.basename(path.rstrip('/\\')), 'name': title, 'id': title_id(title),
                 'root': root, 'work': work, 'candidates': found,
+                'unwrapped': '' if unwrapped == '.' else unwrapped,
                 'size': _tree_size(root), 'warnings': []}
         _choose(plan, found[0]['path'])
         return plan
@@ -466,6 +473,10 @@ def _tree_size(root):
 
 def _choose(plan, rel, runtime=None):
     """Set plan's launch target (a path relative to root) and its runtime."""
+    rel = rel.replace('\\', '/')
+    prefix = plan.get('unwrapped') and plan['unwrapped'] + '/'
+    if prefix and rel.startswith(prefix):
+        rel = rel[len(prefix):]
     target = next((c for c in plan['candidates'] if c['path'] == rel), None)
     if target is None:
         full = os.path.realpath(os.path.join(plan['root'], *rel.replace('\\', '/').split('/')))

ui/index.html

@@ -1169,7 +1169,7 @@ async function sendFiles(files, dirs = new Set()) {
 }
 
 // ---- sideloaded titles (a Linux or Windows build as a Steam Devkit Game) ----
-const RESERVED_IDS = ["steam", "steamdeckard", "steamvr", "steamvrdeckard"];
+const RESERVED_IDS = ["steam", "steamdeckard", "steamvr", "steamvrdeckard", "devkit-steam"];
 function titleId(name) {  // mirrors frame_titles.title_id: the name Steam shows
   const trim = x => x.replace(/^[_-]+|[_-]+$/g, "");
   let s = trim(trim(String(name).trim().replace(/[^A-Za-z0-9_-]+/g, "_").replace(/_+/g, "_")).slice(0, 64));
@@ -1216,7 +1216,9 @@ function confirmTitle(plan, canPush) {
     $("titleSrc").textContent = `${plan.source} · ${gb(plan.size)}`;
     $("titleName").value = plan.name;
     const idNote = () => { const id = titleId($("titleName").value);
-      $("titleIdNote").textContent = id ? `Shows in Steam as ${id}` : "Needs some letters or digits"; };
+      $("titleIdNote").textContent = !id ? "Needs some letters or digits"
+        : installedTitles.has(id.toLowerCase()) ? `Replaces the installed ${id}, and everything in its folder on the Frame`
+        : `Shows in Steam as ${id}`; };
     $("titleName").oninput = idNote; idNote();
     $("titleExe").innerHTML = plan.candidates.map(c =>
       `<option value="${esc(c.path)}"${c.path === plan.target ? " selected" : ""}${c.blocked ? " disabled" : ""}>${esc(candLabel(c))}</option>`).join("");
@@ -1272,10 +1274,12 @@ async function installTitle(token, choice) {
     }
   } finally { $("prog").style.display = "none"; loadTitles(); }
 }
+let installedTitles = new Set();  // lower-case ids, so the install dialog can warn before replacing one
 async function loadTitles() {
   let list;
   try { list = (await api("/api/titles")).titles; }
   catch (e) { $("titleList").innerHTML = `<div class="sub">${esc(e.message)}</div>`; return; }
+  installedTitles = new Set(list.map(t => String(t.id).toLowerCase()));
   $("titleCount").textContent = list.length ? `${list.length}` : "";
   $("titleList").innerHTML = list.length ? list.map(t => `
     <div class="item">