Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large nested blockquote consumes huge amount of memory and crashes PHP #681

Closed
lin-toto opened this issue Nov 24, 2018 · 4 comments
Closed
Projects
Milestone

Comments

@lin-toto
Copy link

lin-toto commented Nov 24, 2018

My site has been experiencing attacks achieved by posting a large nested blockquote in the forum. It looks like this:

>>>>>>>>>>>>>>>>>>>>>>... (about 10k+ blockquote markings in one line)

This eventually exhausts all available memory and kills the PHP instance. I wonder if this is the nature of the parser or is it an issue?

This is sucessfully reproduced in the project demo (http://parsedown.org/demo). (sorry if this caused any damage)

@lin-toto lin-toto changed the title Large nested blockquote consumes huge amount of memory and crashing PHP Large nested blockquote consumes huge amount of memory and crashes PHP Nov 24, 2018
@NightScript370
Copy link

Can reproduce as well.
This is most likely the nature of the parser, though it should be fixed.

@aidantwoods
Copy link
Collaborator

aidantwoods commented Nov 25, 2018

Thanks for reporting. Probably the best way to resolve this is for Parsedown to track and limit parse recursion depth to some sane limit that actual text isn't likely to reach, but also isn't going come close to consuming all of PHP's available memory. We can make this configurable in-case someone wants to lower or raise the limit depending on use cases.

I think in principle you'd still need to have some submission limit when accepting user data though: an attacker could just submit the > character n - 1 times (where n is the recursion limit) and submit it across multiple lines to achieve a similar effect even with a maximum depth in place.

Probably the preferable thing for Parsedown to do if the recursion limit is exceeded is to cease sub-parsing and output potentially deeper remaining text as if it were regular text, though perhaps having Parsedown stop and report failure might be useful too?

@cebe
Copy link
Contributor

cebe commented Nov 27, 2018

this has been reported here: #86 but never got merged.

Probably the preferable thing for Parsedown to do if the recursion limit is exceeded is to cease sub-parsing and output potentially deeper remaining text as if it were regular text

👍

though perhaps having Parsedown stop and report failure might be useful too?

when converting markdown to html there is no failure condition, everything that is not valid markdown will remain plain text. So I think also in this case continuing without error is the best solution.
Afaik this is what other parsers do in such cases. E.g. https://github.com/thephpleague/commonmark/blob/1804ff378ff685b239ef29bdc0b176979f41b064/tests/functional/MaximumNestingLevelTest.php

aidantwoods added a commit that referenced this issue Apr 7, 2019
@aidantwoods aidantwoods added this to the 2.0.0 milestone Apr 7, 2019
@aidantwoods
Copy link
Collaborator

9eb6a02 adds a recursion limiter to the 2.0.x branch. Bear in mind that accepting large input could still cause PHP to crash due to not having enough memory to, for example, copy a string.

@aidantwoods aidantwoods added this to To do in 2.0.0 via automation Apr 8, 2019
@aidantwoods aidantwoods moved this from To do to Done in 2.0.0 Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2.0.0
  
Done
Development

No branches or pull requests

4 participants