highlightjs / highlight.js

JavaScript syntax highlighter with language auto-detection and zero dependencies.
https://highlightjs.org/
BSD 3-Clause "New" or "Revised" License
23.75k stars 3.6k forks source link

Repeated highlighting of already highlighted code causes "*One of your code blocks includes unescaped HTML. This is a potentially serious security risk.*" #3761

Closed Abdull closed 1 year ago

Abdull commented 1 year ago

Describe the issue/behavior that seems buggy

This issue exists at least in the current highlight.js version 11.7.0.

When highlight.js has already highlighted some code, then when this code is highlighted again, highlight.js will warn with "One of your code blocks includes unescaped HTML. This is a potentially serious security risk.".

This e.g. happens with consecutive executions of hljs.highlightAll().

I believe this is a false alarm, as my assumption for the root cause is that highlight.js detects its own previously added <span class="hljs-syntaxfoo>...</span> styling elements.

Sample Code or Instructions to Reproduce

Other highlighting options also provoke this warning, e.g. with hljs.highlightElement(..):

let highlightedCodes = [...document.getElementsByClassName('hljs')];
highlightedCodes.forEach( highlightedCode => hljs.highlightElement(highlightedCode) );

Expected behavior

On consecutive highlighting attempts, highlight.js shall notice already-highlighted code and handle the situation gracefully:

Option 1

Use a marker to convey that the code is already highlighted, e.g. by adding a data attribute such as data-hljs-highlighted="true" to the wrapping <code>...</code> element (<code class="hljs language-foo" data-hljs-highlighted="true">..</code>), and have highlight.js skip highlighting alltogether when it detects such marker.

But this highlighting skipping may not be desirable for scenarios where to-be-highlighted code is dynamically updated, e.g. a poor man's code editor with <pre contenteditable><code>...</code></pre>.

Option 2

Tolerate highlight.js' own styling HTML and suppress warnings for them.

I am not sure if this could unintentionally swallow correct warnings, e.g. when wrapping malicious unescaped HTML within highlight.js syntax highlighting styling elements (<span class="hljs-comment"><script>doMaliciousThings()</script></span>).

Additional context

While consecutive highlighting causes DOM updates to the already-highlighted code, the end result stays the same (i.e., the highlighting is effectively idempotent), which I consider a good thing/expected. E.g., consecutive highlighting does not create undesireable nested <span class="hljs-comment"><span class="hljs-comment">...</span></span> styling tags. 👍

Also see related discussion in https://github.com/highlightjs/highlight.js/issues/2886 ("Detect HTML/JS injection attacks and warn users pro-actively").

joshgoebel commented 1 year ago

This used to be a thing, but the state was hacked onto the DOM object itself... I would accept a patch that added this functionality back by adding a data attribute "tag" (as suggested above) to highlighted elements and then highlightElement would ignore any blocks that were previously tagged in such a fashion (perhaps emitting an "already highlighted" warning - as this is still IMHO broken behavior - apps are expected to handle this concern for themselves).

joshgoebel commented 1 year ago

3987abe437dbf962d64a51da6282d9c9bc20fc13

OmkarMohite505 commented 1 week ago

This solution is working, Try it out

document.querySelectorAll('pre code').forEach((block) => {
      if (block.hasAttribute('data-highlighted')) {

      }
      else{
        hljs.highlightBlock(block as HTMLElement);
      }

    });