fix(subtitles): escape a rejected cue tag instead of deleting it

Deleting a tag whole is what lets a filter like this be reassembled around: strip the `<b>` out of `<scr<b>ipt>`
and the two halves close up into a tag nobody wrote. The leftovers are escaped one character at a time instead,
which also covers `-->` in cue text without a second multi-character replacement.

Both are what CodeQL flagged on the branch, js/incomplete-multi-character-sanitization and js/bad-tag-filter.
Neither was reachable as an injection, because the file is served as text/vtt and a cue is parsed by the WebVTT
cue-text parser rather than as HTML, but a sanitiser that cannot be reassembled around is the cheaper thing to own.

lib/subtitle-validation.ts

@@ -145,19 +145,39 @@ function parseTimingLine(line: string): { start: number; end: number } | null {
   return { start, end };
 }
 
+const CUE_TAG = /<[^<>]*>/g;
+
+/**
+ * Angle brackets outside a recognised tag are escaped one character at a time rather than
+ * the offending tag being deleted whole. Deleting is what lets a filter like this be
+ * reassembled around: strip the `<b>` out of `<scr<b>ipt>` and the two halves close up
+ * into a tag that was never written. Nothing closes up when the leftovers are escaped
+ * instead, and the same escaping takes care of `-->`, which would otherwise be read back
+ * as a timing line and split the cue in two. `&` is left alone so a file that already
+ * spells its entities properly keeps them.
+ */
+function escapeCueText(text: string): string {
+  return text.replace(/</g, '&lt;').replace(/>/g, '&gt;');
+}
+
 function sanitizeCueLine(line: string): string {
-  return (
+  // ASS/SSA override blocks travel in SRT files ripped from other formats. The VTT parser
-    line
+  // renders them as literal text, which is never what the author meant.
-      // ASS/SSA override blocks travel in SRT files ripped from other formats. The VTT
+  const withoutOverrides = line.replace(/\{\\[^}]*\}/g, '');
-      // parser renders them as literal text, which is never what the author meant.
+
-      .replace(/\{\\[^}]*\}/g, '')
+  let sanitized = '';
-      .replace(/<[^<>]*>/g, (tag) => (ALLOWED_CUE_TAGS.some((re) => re.test(tag)) ? tag : ''))
+  let cursor = 0;
-      // A cue text line containing an arrow would be read back as a timing line and split
+  CUE_TAG.lastIndex = 0;
-      // the cue in two. The entity is what the WebVTT parser expects for a literal `>`.
+  for (let match = CUE_TAG.exec(withoutOverrides); match; match = CUE_TAG.exec(withoutOverrides)) {
-      .replace(/-->/g, '--&gt;')
+    sanitized += escapeCueText(withoutOverrides.slice(cursor, match.index));
-      .replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g, '')
+    if (ALLOWED_CUE_TAGS.some((allowed) => allowed.test(match[0]))) {
-      .trimEnd()
+      sanitized += match[0];
-  );
+    }
+    cursor = match.index + match[0].length;
+  }
+  sanitized += escapeCueText(withoutOverrides.slice(cursor));
+
+  return sanitized.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g, '').trimEnd();
 }
 
 /**
@@ -212,7 +232,13 @@ export function parseSubtitleCues(input: string): SubtitleCue[] {
     }
 
     if (timing.end <= timing.start) continue;
-    const text = textLines.join('\n').slice(0, MAX_CUE_TEXT_LENGTH).trim();
+    // The cap can land inside an escape the sanitiser wrote, so a dangling `&lt` tail is
+    // trimmed rather than left for the parser to render as text.
+    const text = textLines
+      .join('\n')
+      .slice(0, MAX_CUE_TEXT_LENGTH)
+      .replace(/&[a-z]{0,5}$/i, '')
+      .trim();
     if (!text) continue;
 
     cues.push({ start: timing.start, end: timing.end, text });

tests/unit/lib/subtitle-validation.test.ts

@@ -144,6 +144,16 @@ describe('parseSubtitleCues', () => {
     expect(cues[0].text).toBe('<i>tilt</i>alert(1)');
   });
 
+  it('escapes the leftovers of a rejected tag so it cannot be reassembled', () => {
+    // Deleting `<b>` out of the middle would close the two halves into a `<script>` that
+    // was never written. Escaping what is left over is what stops that.
+    const cues = parseSubtitleCues(
+      ['00:00:01,000 --> 00:00:02,000', '<scr<b>ipt>alert(1)', ''].join('\n')
+    );
+    expect(cues[0].text).toBe('&lt;scr<b>ipt&gt;alert(1)');
+    expect(cues[0].text).not.toContain('<script');
+  });
+
   it('neutralises an arrow in cue text so the file cannot be re-split', () => {
     const cues = parseSubtitleCues(['00:00:01,000 --> 00:00:02,000', 'a --> b', ''].join('\n'));
     expect(cues[0].text).toBe('a --&gt; b');