ソースを参照

Audit fixes: escape > in attributes, bound inline scans to linear time

Two findings from the conformance/security audit of the markdown engine:

1. mdEscapeAttr replaced '>' with itself instead of '>' (a no-op). Every
   attribute is double-quoted so this was not exploitable, but the info-string
   class attribute emitted a literal '>' inside the quoted value. Now escaped
   like the other three characters.

2. The inline scanner sliced subject to the end of the paragraph at every '<'
   and '&' (autolink match, entity match, mdUnescapeString), making input with
   thousands of raw HTML tags quadratic: 10k <b>x</b> tags took 11.2s. Entity
   references are at most 34 characters (spec 2.3), so a fixed window covers
   them; autolinks cannot contain whitespace, '<' or '>', so a new mdBoundScan
   helper returns the index of the next such character. Identical output for
   all spec/regression cases; the pathological input drops to 102ms.

Verified: regressions 51/51, spec 567/588 in-scope (96.4%), 112-payload
security corpus clean under html.parser, meson build + Vala tests pass.
saddle-Clanker-Saddle 1 ヶ月 前
親
コミット
f0c967dfa4
1 ファイル変更、33 行追加、6 行削除
  1. 33 6
      js/statum.js

+ 33 - 6
js/statum.js

@@ -2232,6 +2232,26 @@
     return null;
   }
   var MD_LINK_DEST_BRACES_RE = /^<(?:[^<>\n\\\x00]|\\.)*>/;
+  /**
+   * End bound for scanning a construct that cannot contain whitespace, '<' or
+   * '>' from `pos` (autolinks; raw HTML tags scan themselves linearly).
+   * Slicing to this index instead of the end of the subject keeps the inline
+   * parser linear on pathological input (many unmatched '<' or '&'): the
+   * anchored construct regexes cannot match past the bound anyway.
+   * @param {string} subject
+   * @param {number} pos
+   * @returns {number}
+   */
+  function mdBoundScan(subject, pos) {
+    var i = pos + 1, c;
+    while (i < subject.length) {
+      c = subject.charCodeAt(i);
+      if (c <= 0x20 || c === 0x3C || c === 0x3E) break;
+      i++;
+    }
+    return i < subject.length ? i + 1 : subject.length;
+  }
+
   /**
    * Match a raw HTML open/close tag at subject[pos] using a linear scan
    * (the equivalent regex backtracks catastrophically on malformed input).
@@ -2285,6 +2305,7 @@
     return null;
   }
 
+
   /**
    * CommonMark's URL normalisation: percent-encode every character that is
    * not in the RFC 3986 "unreserved + reserved + '#' escape" set. Non-ASCII
@@ -2577,9 +2598,11 @@
         continue;
       }
       if (ch === '&') {
-        rep = mdReplaceEntity(s.slice(i));
+        // entity references are at most 34 characters long (spec 2.3), so a
+        // bounded window is enough and keeps this loop linear in s.length
+        rep = mdReplaceEntity(s.slice(i, i + 34));
         if (rep !== null) {
-          m = s.slice(i).match(MD_ENTITY_HERE_RE);
+          m = s.slice(i, i + 34).match(MD_ENTITY_HERE_RE);
           out += rep;
           i += m[0].length;
           continue;
@@ -2945,7 +2968,11 @@
      * Try an autolink at pos. Returns the node or null.
      */
     function parseAutolink() {
-      var rest = subject.slice(pos);
+      // Autolinks cannot contain whitespace or '<'/'>' (MD_AUTOLINK_RE), so
+      // slicing to the next such character bounds the scan without changing
+      // what can match. Without this bound the slice+match was quadratic in
+      // pathological input (thousands of unmatched '<').
+      var rest = subject.slice(pos, mdBoundScan(subject, pos));
       var m = rest.match(MD_AUTOLINK_RE);
       var node;
       if (m) {
@@ -3081,10 +3108,10 @@
           break;
         case '&':
           {
-            var rep = mdReplaceEntity(subject.slice(pos));
+            var rep = mdReplaceEntity(subject.slice(pos, pos + 34));
             if (rep !== null) {
               text(rep);
-              pos += subject.slice(pos).match(MD_ENTITY_HERE_RE)[0].length;
+              pos += subject.slice(pos, pos + 34).match(MD_ENTITY_HERE_RE)[0].length;
             } else {
               text('&');
               pos++;
@@ -3187,7 +3214,7 @@
   /** Escape for a double-quoted attribute value. */
   function mdEscapeAttr(s) {
     return String(s).replace(/&/g, '&amp;').replace(/"/g, '&quot;')
-                    .replace(/</g, '&lt;').replace(/>/g, '>');
+                    .replace(/</g, '&lt;').replace(/>/g, '&gt;');
   }
 
   /**