Merge pull request #2345 from PR0M3TH3AN/fix-magnet-injection-vulnerability-3769700873046071631

🔒 Fix Magnet Parameter Injection via xt Parameter Decoding

js/magnetShared.js

@@ -223,19 +223,25 @@ export function normalizeMagnetInput(rawValue) {
 
   const params = [];
   for (const rawParam of rawParams) {
-    const [rawKey, rawVal = ""] = rawParam.split("=", 2);
-    const key = rawKey.trim();
+    const eqIndex = rawParam.indexOf("=");
+    let key, rawVal;
+    if (eqIndex === -1) {
+      key = rawParam;
+      rawVal = "";
+    } else {
+      key = rawParam.slice(0, eqIndex);
+      rawVal = rawParam.slice(eqIndex + 1);
+    }
+
+    key = key.trim();
     if (!key) {
       continue;
     }
-    const lowerKey = key.toLowerCase();
-    let value = rawVal.trim();
-    if (lowerKey === "xt" && value) {
-      const decoded = safeDecodeURIComponentLoose(value);
-      if (decoded && decoded !== value) {
-        value = decoded;
-        didMutate = true;
-      }
+
+    const trimmedRawVal = rawVal.trim();
+    const value = safeDecodeURIComponentLoose(trimmedRawVal);
+    if (value !== trimmedRawVal) {
+      didMutate = true;
     }
     params.push(createParam(key, value));
   }
@@ -253,7 +259,7 @@ export function normalizeMagnetInput(rawValue) {
 
 export function buildMagnetUri(normalizedScheme, params, fragment) {
   const queryString = params
-    .map(({ key, value }) => (value ? `${key}=${value}` : key))
+    .map(({ key, value }) => (value ? `${key}=${encodeURIComponent(value)}` : key))
     .join("&");
   return `${normalizedScheme}${queryString ? `?${queryString}` : ""}${fragment || ""}`;
 }

tests/edit-modal-submit-state.test.mjs

@@ -254,19 +254,19 @@ test('editing the magnet refreshes torrent hints when ws/xs remain locked', asyn
   );
   assert.ok(
     submission.updatedData.magnet.includes(
-      'urn:btih:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
+      'urn%3Abtih%3Abbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
     ),
-    'normalized magnet should contain the updated info hash',
+    'normalized magnet should contain the updated info hash (encoded)',
   );
   assert.ok(
-    submission.updatedData.magnet.includes('ws=https://new.example/video.mp4'),
-    'normalized magnet should include the refreshed ws hint',
+    submission.updatedData.magnet.includes('ws=https%3A%2F%2Fnew.example%2Fvideo.mp4'),
+    'normalized magnet should include the refreshed ws hint (encoded)',
   );
   assert.ok(
     submission.updatedData.magnet.includes(
-      'xs=https://new.example/video.torrent',
+      'xs=https%3A%2F%2Fnew.example%2Fvideo.torrent',
     ),
-    'normalized magnet should include the refreshed xs hint',
+    'normalized magnet should include the refreshed xs hint (encoded)',
   );
 
   modal.setSubmitState({ pending: false });

tests/video-note-payload.test.mjs

@@ -59,15 +59,18 @@ test("augments magnets with ws/xs hints", () => {
 
   assert.deepEqual(errors, []);
   const { magnet, ws, xs } = payload.legacyFormData;
-  assert.ok(magnet.startsWith("magnet:?xt=urn:btih:"));
+  assert.ok(
+    magnet.startsWith("magnet:?xt=urn%3Abtih%3A"),
+    "Magnet should start with xt parameter (encoded)",
+  );
   assert.ok(
     magnet.includes("ws=https%3A%2F%2Fcdn.example.com%2Fvideo.mp4") ||
       magnet.includes("ws=https://cdn.example.com/video.mp4"),
     "Normalized magnet should include ws hint",
   );
   assert.ok(
-    magnet.includes("xs=https://cdn.example.com/video.torrent") ||
-      magnet.includes("xs=https%3A%2F%2Fcdn.example.com%2Fvideo.torrent"),
+    magnet.includes("xs=https%3A%2F%2Fcdn.example.com%2Fvideo.torrent") ||
+      magnet.includes("xs=https://cdn.example.com/video.torrent"),
     "Normalized magnet should include xs hint",
   );
   assert.equal(ws, "https://cdn.example.com/video.mp4");