Skip to content

Malformed HTML comment closer -- > causes Markdown parser to hang / OOM #1554

Closed

Description

@veggerby

When a Markdown file contains an HTML comment with a malformed closer (-- > instead of -->), the Python-Markdown parser treats the rest of the document as an unterminated comment. This can result in unbounded memory usage and eventual OOM termination. No exception is raised, the process is simply killed by the OS.

Related downstream issue: mkdocs/mkdocs#4030

Environment

  • Python-Markdown: 3.9
  • Python: 3.11.2
  • OS: Ubuntu 22.04 (also reproduces in GitHub Actions ubuntu-latest)

Minimal Reproducer

import markdown

bad = """# Hello

<!-- This comment is malformed and never closes -- >
Some content after the bad comment.
"""

html = markdown.markdown(bad)
print(html)

Expected:

  • Either emit valid HTML with the comment treated as literal text, or raise a parse error.

Actual:

  • The parser goes into runaway behavior: memory usage grows rapidly until the process is killed by the OOM killer. No Python traceback is shown.

Why this happens (likely)
The HTML block parser looks for a --> terminator. If it encounters -- > (space before >), it fails to close the comment and consumes the rest of the document as one giant comment block. This produces extremely large buffers and regex backtracking.

Impact

  • A single malformed -- > can take down CI pipelines or documentation builds.
  • From a security perspective, it’s effectively a denial-of-service vector.

Workarounds

  • Find/replace malformed closers in Markdown sources:

    grep -RIn "\-\- >" docs && echo "Bad comment found"
  • Replace with valid -->.

Suggested fix ideas

  • Fail closed: if no proper --> terminator is found, treat the sequence as literal text instead of an open comment.
  • Optionally normalize --\s*> as a valid closing delimiter (tolerant parser).
  • Impose a max comment block length; if exceeded, abort parsing as a safety guard.

Activity

  1. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    So, I'm not quite sure I am understanding what is being described. You say it can create unbounded memory because it treats the entire document as a comment. And this is specific to the comments only, nothing else? What if I create one giant paragraph, or anything else? What specifically about one giant HTML comment is more problematic than any other giant HTML tag?

  2. veggerby commented on Sep 23, 2025

    @veggerby
    Author

    Essentially if you try to parse "<-- comment -- >" (NB! extra space) if goes into somekind of recursion/infinite loops and fails with out of memory.

    The "Why this happens" is entirely speculative

  3. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    Hmm, this seems specific to Python version. This doesn't happen on 3.13 or 3.14 RC 3.

  4. veggerby commented on Sep 23, 2025

    @veggerby
    Author

    I can confirm I see this as well on 3.13

    import markdown
    
    good = """# Hello
    
    <!-- This comment is well-formed and never closes -->
    Some content after the good comment.
    """
    
    bad = """# Hello
    
    <!-- This comment is malformed and never closes -- >
    Some content after the bad comment.
    """
    
    html = markdown.markdown(good)
    print(html)
    
    html = markdown.markdown(bad)
    print(html)

    output 3.13.3:

    <h1>Hello</h1>
    <!-- This comment is well-formed and never closes -->
    <p>Some content after the good comment.</p>
    <h1>Hello</h1>
    <p>&lt;!-- This comment is malformed and never closes -- &gt;
    Some content after the bad comment.</p>```

    output 3.11.2:

    <h1>Hello</h1>
    <!-- This comment is well-formed and never closes -->
    <p>Some content after the good comment.</p>
    [1]    2806 killed     python3 bad.py

    Takes a few minutes before it dies (consuming approx 9Gb memory in DevContainer)

    But this means there is a security issue with markdown on < 3.13.

  5. mitya57 commented on Sep 23, 2025

    @mitya57
    Collaborator

    I also cannot reproduce this with Python 3.13.7, 3.12.10 or 3.11.9 from Debian.

  6. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    So, there is something different with the parser on lesser versions that keeps us looking at the same comment over and over...

    If you omitt -- > it completes just fine. If you include -->, it completes fine, but for some reason, having -- > makes it fail. We simply didn't have a test for this. I would have assumed having -- > would have been the same as not having any ending...

  7. veggerby commented on Sep 23, 2025

    @veggerby
    Author

    Actually not <!-- This comment is malformed and never closes -- > fails

    But <!-- This comment is malformed and never closes or even <!-- This comment is malformed and never closes - > works (yields &gt; encoded)

  8. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    Yep, that is what I'm saying, the issue is very specific to -- > being present. I'm glad this is fixed in later Python versions, but unfortunately, we have to dig into this issue in older versions.

  9. waylan commented on Sep 23, 2025

    @waylan
    Member

    Thanks for the report.

    As a reminder, user input should never result in an error being raised. Therefore, we use the policy of bad input = bad output. Specifically, we do not attempt to fix invalid raw HTML input to make it valid. We simply return the same invalid input as output. Or in the case that the invalid input is so bad that is confuses the parser, then we don't really care what is returned so long as no errors are raised.

    Presumably the issue reported here stems from a bug in the standard lib HtmlParser, which we use. A number of patches to the lib have been made in recent Python releases (and backported), which would explain why the issue does not exist in the most recent releases of 3.11, 3.12, and 3.13. The issue is that not everyone always updates to the most recent point release. Also, we currently support Python 3.9 and 3.10 which have not had those recent patches backported.

    If a fix for this is actually in the standard lib, we have 2 options:

    1. Monkeypatch the standard lib for versions of Python which do not already have the fix.
    2. Vendor the updated lib until such time as we only support updated versions of Python.
  10. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    Presumably the issue reported here stems from a bug in the standard lib HtmlParser

    Not necessarily, I think our recent fix for handling incomplete comments is behaves differently in this very specific case that we didn't have a test for. I'm not sure if the standard library exhibits any issues if you don't override its logic. Basically, further investigation is required.

  11. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    I think I have a fix:

    diff --git a/markdown/htmlparser.py b/markdown/htmlparser.py
    index 63e5df3..7fcf688 100644
    --- a/markdown/htmlparser.py
    +++ b/markdown/htmlparser.py
    @@ -33,6 +33,8 @@ from typing import TYPE_CHECKING, Sequence
     if TYPE_CHECKING:  # pragma: no cover
         from markdown import Markdown
    
    +# Check for versions of at least Python 3.13
    +PYTHON_313 = (3, 13) <= sys.version_info
    
     # Import a copy of the html.parser lib as `htmlparser` so we can monkeypatch it.
     # Users can still do `from html import parser` and get the default behavior.
    @@ -271,7 +273,7 @@ class HTMLExtractor(htmlparser.HTMLParser):
         def handle_comment(self, data: str):
             # Check if the comment is unclosed, if so, we need to override position
             i = self.line_offset + self.offset + len(data) + 4
    -        if self.rawdata[i:i + 3] != '-->':
    +        if PYTHON_313 and self.rawdata[i:i + 3] != '-->':
                 self.handle_data('<')
                 self.override_comment_update = True
                 return
  12. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    Let me rephrase. It stops it from failing, but it will give different results on older Python versions:

    Py < 3.13

    <h1>Hello</h1>
    <!-- This comment is malformed and never closes -->
    <p>Some content after the bad comment.</p>

    Py >= 3.13

    <h1>Hello</h1>
    <p>&lt;!-- This comment is malformed and never closes -- &gt;
    Some content after the bad comment.</p>
  13. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    I haven't figured out why older versions of Python keep re-evaluating the same comment, coming up with increasing, bogus line numbers.

  14. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    I will note that all current tests pass with the patch, and only this specific case shows different results depending on the Python version.

  15. mitya57 commented on Sep 23, 2025

    @mitya57
    Collaborator

    As I said, I can't reproduce it with Python 3.12.10 and 3.11.9 either. Maybe the behavior depends on Python patch version, not on minor one.

    So we need to either check patch versions, or check some attribute (some regex in html.parser?).

  16. facelessuser commented on Sep 23, 2025

    @facelessuser
    Collaborator

    Yeah, unfortunately, I don't have time to dig into that today. It will be easier if we can figure the root issue as to why it behaves differently. I think that will help us better identify how to proceed.

  17. waylan commented on Sep 24, 2025

    @waylan
    Member

    Our relevant code was added in #1539 in response to python/cpython#135464. That was backported to 3.9 (python/cpython#135486), 3.10 (python/cpython#135485), 3.11 (python/cpython#135484), 3.12 (python/cpython#135483), 3.13 (python/cpython#135482), and 3.14 (python/cpython#135481). However, it appears that that change is currently only included in a release for 3.13 (see 3.13.6 release notes) and presumably 3.14 (as a pre-release). It appears that for all other Python versions, the most recent release was made before the relevant PR was merged. However, that doesn't explain the reported behavior in 3.10 or 3.11.

    Regardless, it is clear that the various bugfixes and backports are not consistently available across all point releases. Perhaps this issue is compounded by some other previous fix which was not backported consistently.

  18. facelessuser commented on Sep 24, 2025

    @facelessuser
    Collaborator

    While I was aware that it wouldn't be available across all versions, testing had indicated the change would only kick in when relevant. The special case of -- > vs --> was not anticipated as the expectation was that -- > would behave as if no --> had been included. This special exception of specific behavior for -- > is what is breaking us.

  19. facelessuser commented on Sep 25, 2025

    @facelessuser
    Collaborator

    I think I have a real, potential fix:

    diff --git a/markdown/htmlparser.py b/markdown/htmlparser.py
    index 63e5df3..658cd37 100644
    --- a/markdown/htmlparser.py
    +++ b/markdown/htmlparser.py
    @@ -33,6 +33,9 @@ from typing import TYPE_CHECKING, Sequence
     if TYPE_CHECKING:  # pragma: no cover
         from markdown import Markdown
    
    +# Included for versions which do not have current comment fix
    +commentclose = re.compile(r'--!?>')
    +commentabruptclose = re.compile(r'-?>')
    
     # Import a copy of the html.parser lib as `htmlparser` so we can monkeypatch it.
     # Users can still do `from html import parser` and get the default behavior.
    @@ -302,6 +305,22 @@ class HTMLExtractor(htmlparser.HTMLParser):
             self.handle_data('<?')
             return i + 2
    
    +    if not hasattr(htmlparser, 'commentabruptclose'):
    +        # Internal -- parse comment, return length or -1 if not terminated
    +        # see https://html.spec.whatwg.org/multipage/parsing.html#comment-start-state
    +        def parse_comment(self, i, report=True):
    +            rawdata = self.rawdata
    +            assert rawdata.startswith('<!--', i), 'unexpected call to parse_comment()'
    +            match = commentclose.search(rawdata, i+4)
    +            if not match:
    +                match = commentabruptclose.match(rawdata, i+4)
    +                if not match:
    +                    return -1
    +            if report:
    +                j = match.start()
    +                self.handle_comment(rawdata[i+4: j])
    +            return match.end()
    +
         def parse_html_declaration(self, i: int) -> int:
             if self.at_line_start() or self.intail:
                 if self.rawdata[i:i+3] == '<![' and not self.rawdata[i:i+9] == '<![CDATA[':
  20. added a commit that references this issue on Sep 25, 2025
    3d72f43
  21. facelessuser commented on Sep 25, 2025

    @facelessuser
    Collaborator

    PR #1555 provides a fix for the infinite loop. We just needed to backport the recent HTML parser changes so that they are available to versions that don't have it.

  22. added a commit that references this issue on Sep 26, 2025
    5354daf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions