GHSA-m7fp-h3p4-hr49 — liquidjs

Summary

The current implementation of strip_html can cause an infinite loop when the input string contains <, has at least one character before <, and no > appears after <.

Details

The problem is in src/filters/html.ts. Specifically, the following part has the infinite loop.

// Raw-text blocks (HTML5) plus '<...>' as the catch-all kind; a regex
// equivalent is O(n^2) in V8 on unclosed openers.
export function strip_html (this: FilterImpl, v: string) {
  const str = stringify(v)
  this.context.memoryLimit.use(str.length)
  const blocks = new Map([['<script', '</script>'], ['<style', '</style>'], ['<!--', '-->'], ['<', '>']])
  let out = ''
  let i = 0
  while (i < str.length) {
    const lt = str.indexOf('<', i)
    if (lt < 0) return out + str.slice(i)
    out += str.slice(i, lt)
    for (const [opener, closer] of blocks) {
      if (!str.startsWith(opener, lt)) continue
      const e = str.indexOf(closer, lt + opener.length)
      if (e >= 0) { i = e + closer.length; break }
      blocks.delete(opener)
    }
    if (i === lt) return out + str.slice(lt)
  }
  return out
}

For the input "a<", the variable lt is updated to 1 by const lt = str.indexOf('<', i). However, the variable i is never updated from its initial value of 0. This is because in const e = str.indexOf(closer, lt + opener.length), e becomes -1, since there is no > after <. Therefore, when execution reaches if (i === lt) return out + str.slice(lt), i is 0. This is the same state as at the beginning of the loop. As a result, the same thing is repeated again from that state, causing an infinite loop.

PoC

const { Liquid } = require('liquidjs');

const engine = new Liquid();

engine.parseAndRender('{{ html | strip_html }}', {
  html: 'a<'
}).then(console.log);

console.log("This is never displayed.");

Impact

This is an infinite loop vulnerability (cf. https://cwe.mitre.org/data/definitions/835.html). This results in a denial of service (DoS). Although a ReDoS vulnerability has previously been reported in the affected function (cf. https://github.com/harttle/liquidjs/security/advisories/GHSA-r7g9-xpmj-5fcq), this issue can cause a more severe impact than that ReDoS vulnerability with an input of only two characters at minimum.

Recommended Fix

There is an issue with the following conditional branch.

if (i === lt) return out + str.slice(lt);

The following should fix the issue.

if (i <= lt) return out + str.slice(lt);