Reproducer for CVE-2020-26298?

[utkarsh2102]

Hello @robin850,

I am taking a look at CVE-2020-26298, which is fixed by a699c82. But I am not sure how to reproduce the issue or anything.

Do you happen to have the POC as well?
CC: @johan-smits :)

[johan-smits]

@utkarsh2102 I do, contact me at my Matrix address: @johan.smits:smitsmail.net and I will share it or by email.

[utkarsh2102]

Thanks, Johan. You should have an email from my end. Please let me know if you did not receive any.

[johan-smits]

Got is and replied.

[utkarsh2102]

Hi again @robin850 and @johan-smits,

I hit an obstacle. I am trying to backport this patch to Debian Jessie (ELTS) where the version of markdown is 3.1.2. And the tests (with this patch) fails with the following error:

..................................E
===============================================================================
Error: test_quote_flag_honors_escape_html(MarkdownTest)
  NoMethodError: undefined method `render' for #<MarkdownTest:0x000000018e8a68>
/<<PKGBUILDDIR>>/test/markdown_test.rb:215:in `test_quote_flag_honors_escape_html'
     212:   def test_quote_flag_honors_escape_html
     213:     text = 'We are not "<svg/onload=pwned>"'
     214: 
  => 215:     output_enabled  = render(text, with: [:quote, :escape_html])
     216:     output_disabled = render(text, with: [:quote])
     217: 
     218:     assert_equal "<p>We are not <q>&lt;svg/onload=pwned&gt;</q></p>", output_enabled
===============================================================================
...........................................

Finished in 0.04121103 seconds.

78 tests, 111 assertions, 0 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications
98.7179% passed

..which is obvious, because it was added in later versions (right?).
So one way I can think of is to also backport lib/redcarpet/cli.rb and fix bin/redcarpet as per version 3.3.4, probably.

Do you have any alternative in mind? Maybe if the code in the patch sanctioned is good (just the ext/redcarpet/html.c part), that is, compatible with v3.1.2, then I can just backport that and not backport the tests. Do you think that'd be OK? Or what do you think!?

[johan-smits]

@utkarsh2102 I personally was not involved in the patch and only in reporting the issue. So you have wait for @robin850 or someone else.

[utkarsh2102]

Aah, sure! @robin850, whenever you have time, could you please take a look at it?

[utkarsh2102]

Hello,

..which is obvious, because it was added in later versions (right?).
So one way I can think of is to also backport lib/redcarpet/cli.rb and fix bin/redcarpet as per version 3.3.4, probably.

Do you have any alternative in mind? Maybe if the code in the patch sanctioned is good (just the ext/redcarpet/html.c part), that is, compatible with v3.1.2, then I can just backport that and not backport the tests. Do you think that'd be OK? Or what do you think!?

Upon taking a more thorough look, it seems like the code in ext/redcarpet/html.c is good enough. It's just that the tests are a problem since they use the render method, which is not present in v3.1.2.

So I am inclined towards just backporting the main code and not the tests. Alternatively, is there a way to rewrite the tests in such a way that doesn't use the render method? Something simple which can be used in v3.1.2?

[utkarsh2102]

Oh wait, I was badly mistaken. The way to call the render method is different in the previous version.
I am not sure how to adapt the tests, could you take a look and let me know?

Thanks in advance! :)

[utkarsh2102]

I tried the following diff:

--- a/test/markdown_test.rb
+++ b/test/markdown_test.rb
@@ -209,6 +209,16 @@
     assert output.include? '<q>quote</q>'
   end
 
+  def test_quote_flag_honors_escape_html
+    text = 'We are not "<svg/onload=pwned>"'
+
+    output_enabled  = render_with(@rndr[:quote, :escape_html], text)
+    output_disabled = render_with(@rndr[:quote], text)
+
+    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
 This is a simple test

but it failed with:

===============================================================================
Error: test_quote_flag_honors_escape_html(MarkdownTest)
  NoMethodError: undefined method `[]' for nil:NilClass
/<<PKGBUILDDIR>>/test/markdown_test.rb:215:in `test_quote_flag_honors_escape_html'
     212:   def test_quote_flag_honors_escape_html
     213:     text = 'We are not "<svg/onload=pwned>"'
     214: 
  => 215:     output_enabled  = render_with(@rndr[:quote, :escape_html], text)
     216:     output_disabled = render_with(@rndr[:quote], text)
     217: 
     218:     assert_equal "<p>We are not <q>&lt;svg/onload=pwned&gt;</q></p>", output_enabled
===============================================================================

Probably I am running in circles, heh. Best to wait for @robin850 here! :)

[utkarsh2102]

Aaaaaaaaaaaaaaah! I made some progress:

--- a/test/markdown_test.rb
+++ b/test/markdown_test.rb
@@ -209,6 +209,16 @@
     assert output.include? '<q>quote</q>'
   end
 
+  def test_quote_flag_honors_escape_html
+    text = 'We are not "<svg/onload=pwned>"'
+
+    output_enabled  = render_with({:quote => true, :escape_html => true}, text)
+    output_disabled = render_with({:quote => true}, text)
+
+    assert_equal "<p>We are not <q><svg/onload=pwned></q></p>\n", output_enabled
+    assert_equal "<p>We are not <q><svg/onload=pwned></q></p>\n", output_disabled
+  end
+
   def test_that_fenced_flag_works
     text = <<fenced
 This is a simple test

With this, the tests pass. But, it looks like :escape_html isn't working as it should, really. 'Cause it should've been <p>We are not <q>&lt;svg/onload=pwned&gt;</q></p>\n in output_enabled instead :/

[utkarsh2102]

Okay, sorry for the spam. But lastly, I guess {:quote => true, :escape_html => true} isn't really working because just with {:escape_html => true} I get: &quot;<svg/onload=pwned>&quot;</p>.

So just the first element of the hash seems to be working, sigh.

[robin850]

Hello there, sorry for the late reply !

..which is obvious, because it was added in later versions (right?).

Yes, it looks like it has been added in version 3.3.0.

The problem raised in your last message is due to the fact that render_with only passes Markdown options, not the renderer's one. Here :escape_html is an option for Redcarpet::Render::HTML ; you should rather try something like:

enabled = Redcarpet::Markdown.new(
  Redcarpet::Render::HTML.new(escape_html: true), quote: true)
disabled = Redcarpet::Markdown.new(Redcarpet::Render::HTML, quote: true)

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

output_enabled  = enabled.render(text)
output_disabled = disabled.render(text)

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

(Actually, maybe this test would be in a better place in the renderer's test file 🤷‍♂️ )

I wasn't aware that such old versions of Redcarpet could still be in the wild. Feel free to ask if you still have troubles ; I will try to answer a bit more quickly.

[utkarsh2102]

Hi @robin850 o/

The problem raised in your last message is due to the fact that render_with only passes Markdown options, not the renderer's one. Here :escape_html is an option for Redcarpet::Render::HTML ; you should rather try something like:

enabled = Redcarpet::Markdown.new(
  Redcarpet::Render::HTML.new(escape_html: true), quote: true)
disabled = Redcarpet::Markdown.new(Redcarpet::Render::HTML, quote: true)

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

output_enabled  = enabled.render(text)
output_disabled = disabled.render(text)

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

Wow, aren't you just amazing? 😭
Thanks a bunch, this works! \o/

I wasn't aware that such old versions of Redcarpet could still be in the wild. Feel free to ask if you still have troubles ; I will try to answer a bit more quickly.

Oh yeah, actually I help maintain redcarpet in Debian. And for Debian oldstable and oldoldstable releases, we had to backort the fix as a part of Debian LTS and ELTS maintenance. Thus had to work this out! :)

But thank you so much for your help! ❤️