Skip to content

Commit 59aebbf

Browse files
yasulabyasulab
committed
Revert "Merge pull request #1740 from coderdojo-japan/fix/brakeman-xss-warnings"
This reverts commit 79d82d4, reversing changes made to 2bc0d97.
1 parent 74a3ed3 commit 59aebbf

File tree

7 files changed

+193
-29
lines changed

7 files changed

+193
-29
lines changed

‎app/helpers/application_helper.rb

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,6 @@ def page_lang(lang)
4848
lang.empty? ? 'ja' : lang
4949
end
5050

51-
def sanitize_content(content)
52-
sanitize(content,
53-
tags: ActionView::Base.sanitized_allowed_tags + ['center'],
54-
attributes: ActionView::Base.sanitized_allowed_attributes + ['id']
55-
)
56-
end
57-
58-
def safe_dojo_url(dojo)
59-
return '#' if dojo.url.blank?
60-
61-
begin
62-
uri = URI.parse(dojo.url)
63-
uri.scheme&.match?(/\Ahttps?\z/) ? dojo.url : '#'
64-
rescue URI::InvalidURIError
65-
'#'
66-
end
67-
end
68-
6951
# 'inline_' プレフィックスがついたflashメッセージをビュー内で表示するヘルパー
7052
# inline_alert → alert, inline_warning → warning のように変換してBootstrapのCSSクラスを適用
7153
def render_inline_flash_messages

‎app/models/dojo.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def active?
9797
def active_at?(date)
9898
created_at <= date && (inactivated_at.nil? || inactivated_at > date)
9999
end
100-
100+
101101
# 再活性化メソッド
102102
def reactivate!
103103
if inactivated_at.present?

‎app/views/docs/show.html.erb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
<div class='container' style='line-height: 1.9em;'>
1111
<section class='doc' style='padding: 50px 0px 100px 0px;'>
12-
<%= sanitize_content(@content) %>
12+
<%= raw @content %>
1313
</section>
1414

1515
<% if request.path.start_with? '/docs' %>
@@ -22,3 +22,5 @@
2222
<%= render 'shared/social_buttons' %>
2323
</section>
2424
</div>
25+
26+

‎app/views/podcasts/show.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
<iframe class="lazyload" src="https://anchor.fm/coderdojo-japan/embed/episodes/<%= @episode.permalink %>" width="100%" scrolling="no" frameborder="yes" style='margin-bottom: 30px;'></iframe>
3838

39-
<%= sanitize_content(Rinku.auto_link(@content)) %>
39+
<%= raw Rinku.auto_link(@content) %>
4040
</div>
4141
</section>
4242

‎app/views/shared/_dojo.html.erb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
<li class="dojo" id="<%= html_escape(dojo.name) %>">
1+
<li class="dojo" id="<%= dojo.name %>">
22
<header>
3-
<%= link_to lazy_image_tag(dojo.logo, alt: html_escape("CoderDojo #{dojo.name}"), class: 'dojo-picture'), safe_dojo_url(dojo),
4-
target: "_blank" %>
3+
<%= link_to lazy_image_tag(dojo.logo, alt: "CoderDojo #{dojo.name}", class: 'dojo-picture'), dojo.url,
4+
target: "_blank", rel: "external noopener" %>
55
<span class="dojo-name">
6-
<%= link_to html_escape("#{dojo.name} (#{dojo.prefecture.name})"), safe_dojo_url(dojo), target: "_blank" %>
6+
<%= link_to "#{dojo.name} (#{dojo.prefecture.name})", dojo.url, target: "_blank", rel: "external noopener" %>
77
<% if not dojo.counter == 1 %>
88
<span class="dojo-counter"
99
data-original-title="道場数"
@@ -15,7 +15,7 @@
1515

1616
<ul class="tags">
1717
<% dojo.tags.first(5).each do |tag| %>
18-
<li><%= html_escape(tag) %></li>
18+
<li><%= tag %></li>
1919
<% end %>
2020

2121
<% if dojo.tags.length > 5 %>
@@ -26,12 +26,12 @@
2626
<div class="tooltip-arrow"></div>
2727
<div class="tooltip-inner"></div>
2828
</div>'
29-
title="<%= html_escape(dojo.tags[5..].join(', ')) %>">...</li>
29+
title="<%= dojo.tags[5..].join(', ') %>">...</li>
3030
<% end %>
3131
</ul>
3232

3333
<p class="dojo-description">
34-
<%= html_escape(dojo.description) %>
34+
<%= dojo.description %>
3535
<% if dojo.is_private %>
3636
<%= link_to 'Private', doc_path('private-dojo'), class: 'dojo-private' %>
3737
<% end %>

‎config/brakeman.ignore

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
{
2+
"ignored_warnings": [
3+
{
4+
"warning_type": "Cross-Site Scripting",
5+
"warning_code": 4,
6+
"fingerprint": "8ba988098c444755698e4e65d38a94f4095948c1a9bc6220c7e2a4636c3c04d7",
7+
"check_name": "LinkToHref",
8+
"message": "Potentially unsafe model attribute in `link_to` href",
9+
"file": "app/views/shared/_dojo.html.erb",
10+
"line": 6,
11+
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
12+
"code": "link_to(\"#{Dojo.new.name} (#{Dojo.new.prefecture.name})\", Dojo.new.url, :target => \"_blank\", :rel => \"external noopener\")",
13+
"render_path": [
14+
{
15+
"type": "controller",
16+
"class": "HomeController",
17+
"method": "show",
18+
"line": 7,
19+
"file": "app/controllers/home_controller.rb",
20+
"rendered": {
21+
"name": "home/show",
22+
"file": "app/views/home/show.html.erb"
23+
}
24+
},
25+
{
26+
"type": "template",
27+
"name": "home/show",
28+
"line": 161,
29+
"file": "app/views/home/show.html.erb",
30+
"rendered": {
31+
"name": "shared/_dojos",
32+
"file": "app/views/shared/_dojos.html.erb"
33+
}
34+
},
35+
{
36+
"type": "template",
37+
"name": "shared/_dojos",
38+
"line": 2,
39+
"file": "app/views/shared/_dojos.html.erb",
40+
"rendered": {
41+
"name": "shared/_dojo",
42+
"file": "app/views/shared/_dojo.html.erb"
43+
}
44+
}
45+
],
46+
"location": {
47+
"type": "template",
48+
"template": "shared/_dojo"
49+
},
50+
"user_input": "Dojo.new.url",
51+
"confidence": "Weak",
52+
"cwe_id": [
53+
79
54+
],
55+
"note": ""
56+
},
57+
{
58+
"warning_type": "Cross-Site Scripting",
59+
"warning_code": 2,
60+
"fingerprint": "b22a549fb14a7e6b3a9c34991ffcacd354dc768d74d50a8f6901e23c3ea19538",
61+
"check_name": "CrossSiteScripting",
62+
"message": "Unescaped model attribute",
63+
"file": "app/views/podcasts/show.html.erb",
64+
"line": 39,
65+
"link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
66+
"code": "Rinku.auto_link(Kramdown::Document.new(self.convert_shownote(Podcast.find_by(:id => params[:id]).content), :input => \"GFM\").to_html)",
67+
"render_path": [
68+
{
69+
"type": "controller",
70+
"class": "PodcastsController",
71+
"method": "show",
72+
"line": 31,
73+
"file": "app/controllers/podcasts_controller.rb",
74+
"rendered": {
75+
"name": "podcasts/show",
76+
"file": "app/views/podcasts/show.html.erb"
77+
}
78+
}
79+
],
80+
"location": {
81+
"type": "template",
82+
"template": "podcasts/show"
83+
},
84+
"user_input": "Podcast.find_by(:id => params[:id]).content",
85+
"confidence": "Weak",
86+
"cwe_id": [
87+
79
88+
],
89+
"note": ""
90+
},
91+
{
92+
"warning_type": "Cross-Site Scripting",
93+
"warning_code": 4,
94+
"fingerprint": "b29f98f4da690ffb7c663390c21db3a71174dae17d06234deab9d6655af6babe",
95+
"check_name": "LinkToHref",
96+
"message": "Potentially unsafe model attribute in `link_to` href",
97+
"file": "app/views/shared/_dojo.html.erb",
98+
"line": 3,
99+
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
100+
"code": "link_to(lazy_image_tag(Dojo.new.logo, :alt => (\"CoderDojo #{Dojo.new.name}\"), :class => \"dojo-picture\"), Dojo.new.url, :target => \"_blank\", :rel => \"external noopener\")",
101+
"render_path": [
102+
{
103+
"type": "controller",
104+
"class": "HomeController",
105+
"method": "show",
106+
"line": 7,
107+
"file": "app/controllers/home_controller.rb",
108+
"rendered": {
109+
"name": "home/show",
110+
"file": "app/views/home/show.html.erb"
111+
}
112+
},
113+
{
114+
"type": "template",
115+
"name": "home/show",
116+
"line": 161,
117+
"file": "app/views/home/show.html.erb",
118+
"rendered": {
119+
"name": "shared/_dojos",
120+
"file": "app/views/shared/_dojos.html.erb"
121+
}
122+
},
123+
{
124+
"type": "template",
125+
"name": "shared/_dojos",
126+
"line": 2,
127+
"file": "app/views/shared/_dojos.html.erb",
128+
"rendered": {
129+
"name": "shared/_dojo",
130+
"file": "app/views/shared/_dojo.html.erb"
131+
}
132+
}
133+
],
134+
"location": {
135+
"type": "template",
136+
"template": "shared/_dojo"
137+
},
138+
"user_input": "Dojo.new.url",
139+
"confidence": "Weak",
140+
"cwe_id": [
141+
79
142+
],
143+
"note": ""
144+
},
145+
{
146+
"warning_type": "Cross-Site Scripting",
147+
"warning_code": 2,
148+
"fingerprint": "e4187193a881ef4e98b77f205c86fcafbef3d65d9269bba30e8612f6a59273ed",
149+
"check_name": "CrossSiteScripting",
150+
"message": "Unescaped model attribute",
151+
"file": "app/views/docs/show.html.erb",
152+
"line": 12,
153+
"link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
154+
"code": "Kramdown::Document.new(Document.new(params[:id]).content, :input => \"GFM\").to_html",
155+
"render_path": [
156+
{
157+
"type": "controller",
158+
"class": "DocsController",
159+
"method": "show",
160+
"line": 42,
161+
"file": "app/controllers/docs_controller.rb",
162+
"rendered": {
163+
"name": "docs/show",
164+
"file": "app/views/docs/show.html.erb"
165+
}
166+
}
167+
],
168+
"location": {
169+
"type": "template",
170+
"template": "docs/show"
171+
},
172+
"user_input": "Document.new(params[:id]).content",
173+
"confidence": "Weak",
174+
"cwe_id": [
175+
79
176+
],
177+
"note": ""
178+
}
179+
],
180+
"brakeman_version": "7.1.0"
181+
}

‎spec/requests/docs_spec.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
get doc_path(param)
1515
doc = Document.new(param)
1616
expected = Kramdown::Document.new(doc.content, input: 'GFM').to_html
17-
expected = ApplicationController.helpers.sanitize_content(expected)
1817
expect(response.body).to include(expected.strip)
1918
end
2019

0 commit comments

Comments
 (0)