Skip to content

Commit

Permalink
Fix a security issue using :quote with :escape_html
Browse files Browse the repository at this point in the history
Reported by @johan-smits.
  • Loading branch information
robin850 committed Dec 15, 2020
1 parent 6270d6b commit a699c82
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 4 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## Version 3.5.1 (Security)

* Fix a security vulnerability using `:quote` in combination with the
`:escape_html` option.

Reported by *Johan Smits*.

## Version 3.5.0

* Avoid mutating the options hash passed to a render object.
Expand Down
9 changes: 8 additions & 1 deletion ext/redcarpet/html.c
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,15 @@ rndr_quote(struct buf *ob, const struct buf *text, void *opaque)
if (!text || !text->size)
return 0;

struct html_renderopt *options = opaque;

BUFPUTSL(ob, "<q>");
bufput(ob, text->data, text->size);

if (options->flags & HTML_ESCAPE)
escape_html(ob, text->data, text->size);
else
bufput(ob, text->data, text->size);

BUFPUTSL(ob, "</q>");

return 1;
Expand Down
2 changes: 1 addition & 1 deletion lib/redcarpet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
require 'redcarpet/compat'

module Redcarpet
VERSION = '3.5.0'
VERSION = '3.5.1'

class Markdown
attr_reader :renderer
Expand Down
4 changes: 2 additions & 2 deletions redcarpet.gemspec
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# encoding: utf-8
Gem::Specification.new do |s|
s.name = 'redcarpet'
s.version = '3.5.0'
s.version = '3.5.1'
s.summary = "Markdown that smells nice"
s.description = 'A fast, safe and extensible Markdown to (X)HTML parser'
s.date = '2019-07-29'
s.date = '2020-12-15'
s.email = 'vicent@github.com'
s.homepage = 'http://github.com/vmg/redcarpet'
s.authors = ["Natacha Porté", "Vicent Martí"]
Expand Down
10 changes: 10 additions & 0 deletions test/markdown_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,16 @@ def test_quote_flag_works
assert_equal '<p>this is a <q>quote</q></p>', output
end

def test_quote_flag_honors_escape_html
text = 'We are not "<svg/onload=pwned>"'

output_enabled = render(text, with: [:quote, :escape_html])
output_disabled = render(text, with: [:quote])

assert_equal "<p>We are not <q>&lt;svg/onload=pwned&gt;</q></p>", output_enabled
assert_equal "<p>We are not <q><svg/onload=pwned></q></p>", output_disabled
end

def test_that_fenced_flag_works
text = <<-fenced.strip_heredoc
This is a simple test
Expand Down

4 comments on commit a699c82

@reedloden
Copy link

Choose a reason for hiding this comment

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

@robin850 Is there a CVE or GHSA assigned to this?

@robin850
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@reedloden : No, the decision was taken not to request a CVE as the process can be tedious. Also, as I'm not an owner on this repository, I can't create a GHSA here.

@reedloden
Copy link

Choose a reason for hiding this comment

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

@robin850
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@reedloden Oh, thank you very much for handling this!

Please sign in to comment.