Project

General

Profile

Actions

Defect #36958

closed

Crafted input breaks CommonMark Markdown formatter

Added by Go MAEDA almost 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Text formatting
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

If you create an issue or a Wiki page contains specific data, the CommonMark Markdown formatter raises an exception when rendering the object. Malicious users can use this bug for DoS attacks.

Steps to reproduce:

1. Set the text formatting to "CommonMark Markdown"
2. Create an issue that contains a string http://example.com/foo#bar#
3. Access the newly created issue. You will see "Internal Error"

ActionView::Template::Error (bad URI(is not URI?): "http://example.com/foo#bar#"):
    88:
    89:   <p><strong><%=l(:field_description)%></strong></p>
    90:   <div class="wiki">
    91:   <%= textilizable @issue, :description, :attachments => @issue.attachments %>
    92:   </div>
    93: </div>
    94: <% end %>

lib/redmine/wiki_formatting/common_mark/external_links_filter.rb:34:in `block in call'
lib/redmine/wiki_formatting/common_mark/external_links_filter.rb:29:in `call'
lib/redmine/wiki_formatting/common_mark/formatter.rb:66:in `to_html'
lib/redmine/wiki_formatting.rb:96:in `to_html'
app/helpers/application_helper.rb:868:in `textilizable'
app/views/issues/show.html.erb:91
app/controllers/issues_controller.rb:118:in `block (2 levels) in show'
app/controllers/issues_controller.rb:110:in `show'
lib/redmine/sudo_mode.rb:61:in `sudo_mode'

Files

36958.patch (1.55 KB) 36958.patch Go MAEDA, 2022-04-14 14:50
Actions #1

Updated by Go MAEDA almost 2 years ago

the attached patch fixes the issue.

Actions #2

Updated by Go MAEDA almost 2 years ago

  • Target version set to 5.0.1

Setting the target version to 5.0.1.

Actions #3

Updated by Holger Just almost 2 years ago

I can confirm this issue. However, I don't believe this can be used as an actual DoS of the application itself. This issue might be abused however to cause errors on many pages by including the invalid URI in issues / journals / wiki pages, ....

With that being said, I think we could also just use the following code instead of rescuing the specific exception. If you are confident that URI::InvalidURIError is the only exception being thrown for invalid URIs, then your code is fine too.

scheme = URI.parse(url).scheme rescue nil
next if scheme.blank?
Actions #4

Updated by Marius BĂLTEANU almost 2 years ago

  • Assignee set to Go MAEDA

Go MAEDA, can you handle this?

Actions #5

Updated by Marius BĂLTEANU almost 2 years ago

  • Status changed from New to Resolved
  • Assignee changed from Go MAEDA to Marius BĂLTEANU
  • Resolution set to Fixed

Patches committed with the recommendation from Holger. I think it's safe to not consider this a security issue.

Actions #6

Updated by Marius BĂLTEANU almost 2 years ago

  • Status changed from Resolved to Closed
Actions #7

Updated by Marius BĂLTEANU almost 2 years ago

  • Project changed from 2 to Redmine
Actions #8

Updated by Marius BĂLTEANU almost 2 years ago

  • Category set to Text formatting
Actions

Also available in: Atom PDF