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

Link label security #1515

Merged
merged 3 commits into from
Jul 4, 2019
Merged

Link label security #1515

merged 3 commits into from
Jul 4, 2019

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 styfle, davisjam and joshbruce 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.

@@ -1,3 +1,3 @@
[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 <sheogorath@shivering-isles.com>
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 <sheogorath@shivering-isles.com>
@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