Skip to content

Conversation

@UziTech
Copy link
Member

@UziTech UziTech commented Jul 2, 2019

Marked version: master

Description

Fixes the ReDOS vulnerability in #1493

This does break a current test with a single backtick in a link label

[the ` character](/url)

That markdown can be fixed by escaping the backtick

[the \` character](/url)

Fixes #1493

Contributor

  • Test(s) exist to ensure functionality and minimize regression

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@UziTech UziTech added category: links L0 - security A security vulnerability within the Marked library is discovered labels Jul 2, 2019
@UziTech UziTech requested review from davisjam, joshbruce and styfle July 2, 2019 16:53
@UziTech
Copy link
Member Author

UziTech commented Jul 2, 2019

If this PR is merged I will create a new issue about the single backtick so we can still track it.

@UziTech
Copy link
Member Author

UziTech commented Jul 2, 2019

This is a temporary fix. Links will need to be parsed with a function to be fully spec compliant to allow the single backtick along with nested brackets like we are parsing the href.

[the `]` character](/url)

[the ` character](/url)
[the \` character](/url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this PR is going to introduce a regression but fix a redos issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Contributor

@davisjam davisjam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@UziTech UziTech merged commit 0ee3aa9 into markedjs:master Jul 4, 2019
@UziTech UziTech mentioned this pull request Jul 5, 2019
12 tasks
SISheogorath added a commit to hedgedoc/hedgedoc that referenced this pull request Aug 15, 2019
Meta-marked 0.4.4 which we used from our git repository contains a
RegexDOS attack in the marked dependency. The dependency was already
updated in our meta-marked repository, but not updated in yarn.

This made us still vulnerable to this ReDOS which was able to cause a
DOS attack on the server when updating a note.

For Details:

https://github.com/markedjs/marked/releases/tag/v0.7.0
markedjs/marked#1515

What is a ReDOS?

A ReDOS attack is a DOS attack where an attacker targets a
not-well-written Regular Expression. Regular expressions try to build a
tree of all possibilities it can match in order to figure out if the
given statement is valid or not. A ReDOS attack abuses this concept by
providing a statement that doesn't match but causes extremly huge trees
that simply lead to exhausting CPU usage.

For more details see: https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS

Credit:

Huge thanks to @bitinerant for finding this and handling it with a
responsible disclosure.

Also thanks to the `marked`-team for fixing things already.

Signed-off-by: Sheogorath <[email protected]>
SISheogorath added a commit to SISheogorath/codimd-server that referenced this pull request Sep 5, 2019
Meta-marked 0.4.4 which we used from our git repository contains a
RegexDOS attack in the marked dependency. The dependency was already
updated in our meta-marked repository, but not updated in yarn.

This made us still vulnerable to this ReDOS which was able to cause a
DOS attack on the server when updating a note.

For Details:

https://github.com/markedjs/marked/releases/tag/v0.7.0
markedjs/marked#1515

What is a ReDOS?

A ReDOS attack is a DOS attack where an attacker targets a
not-well-written Regular Expression. Regular expressions try to build a
tree of all possibilities it can match in order to figure out if the
given statement is valid or not. A ReDOS attack abuses this concept by
providing a statement that doesn't match but causes extremly huge trees
that simply lead to exhausting CPU usage.

For more details see: https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS

Credit:

Huge thanks to @bitinerant for finding this and handling it with a
responsible disclosure.

Also thanks to the `marked`-team for fixing things already.

Signed-off-by: Sheogorath <[email protected]>
@UziTech UziTech deleted the link-label-security branch September 11, 2019 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: links L0 - security A security vulnerability within the Marked library is discovered

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extremely slow processing on malformed markdown (0.6.2)

3 participants