Merge pull request #1740 from coderdojo-japan/fix/brakeman-xss-warnings

Cross-Site Scripting警告の解消

Author: yasulab

app/helpers/application_helper.rb

@@ -48,6 +48,24 @@ def page_lang(lang)
     lang.empty? ? 'ja' : lang
   end
 
+  def sanitize_content(content)
+    sanitize(content, 
+      tags: ActionView::Base.sanitized_allowed_tags + ['center'], 
+      attributes: ActionView::Base.sanitized_allowed_attributes + ['id']
+    )
+  end
+
+  def safe_dojo_url(dojo)
+    return '#' if dojo.url.blank?
+
+    begin
+      uri = URI.parse(dojo.url)
+      uri.scheme&.match?(/\Ahttps?\z/) ? dojo.url : '#'
+    rescue URI::InvalidURIError
+      '#'
+    end
+  end
+
   # 'inline_' プレフィックスがついたflashメッセージをビュー内で表示するヘルパー
   # inline_alert → alert, inline_warning → warning のように変換してBootstrapのCSSクラスを適用
   def render_inline_flash_messages

app/models/dojo.rb

@@ -94,7 +94,7 @@ def active?
   def active_at?(date)
     created_at <= date && (inactivated_at.nil? || inactivated_at > date)
   end
-  
+
   # 再活性化メソッド
   def reactivate!
     if inactivated_at.present?

app/views/docs/show.html.erb

@@ -9,7 +9,7 @@
 
 <div class='container' style='line-height: 1.9em;'>
   <section class='doc' style='padding: 50px 0px 100px 0px;'>
-    <%= raw @content %>
+    <%= sanitize_content(@content) %>
   </section>
 
   <% if request.path.start_with? '/docs' %>
@@ -22,5 +22,3 @@
     <%= render 'shared/social_buttons' %>
   </section>
 </div>
-
-

app/views/podcasts/show.html.erb

@@ -36,7 +36,7 @@
 
       <iframe class="lazyload" src="https://anchor.fm/coderdojo-japan/embed/episodes/<%= @episode.permalink %>" width="100%" scrolling="no" frameborder="yes" style='margin-bottom: 30px;'></iframe>
 
-      <%= raw Rinku.auto_link(@content) %>
+      <%= sanitize_content(Rinku.auto_link(@content)) %>
     </div>
   </section>
 

app/views/shared/_dojo.html.erb

@@ -1,9 +1,9 @@
-<li class="dojo" id="<%= dojo.name %>">
+<li class="dojo" id="<%= html_escape(dojo.name) %>">
   <header>
-    <%= link_to lazy_image_tag(dojo.logo, alt: "CoderDojo #{dojo.name}", class: 'dojo-picture'), dojo.url,
-              target: "_blank", rel: "external noopener" %>
+    <%= link_to lazy_image_tag(dojo.logo, alt: html_escape("CoderDojo #{dojo.name}"), class: 'dojo-picture'), safe_dojo_url(dojo),
+              target: "_blank" %>
     <span class="dojo-name">
-      <%= link_to "#{dojo.name} (#{dojo.prefecture.name})", dojo.url, target: "_blank", rel: "external noopener" %>
+      <%= link_to html_escape("#{dojo.name} (#{dojo.prefecture.name})"), safe_dojo_url(dojo), target: "_blank" %>
       <% if not dojo.counter == 1 %>
         <span class="dojo-counter"
               data-original-title="道場数"
@@ -15,7 +15,7 @@
 
   <ul class="tags">
     <% dojo.tags.first(5).each do |tag| %>
-      <li><%= tag %></li>
+      <li><%= html_escape(tag) %></li>
     <% end %>
 
     <% if dojo.tags.length > 5 %>
@@ -26,12 +26,12 @@
 	   	           <div class="tooltip-arrow"></div>
 		           <div class="tooltip-inner"></div>
 		         </div>'
-          title="<%= dojo.tags[5..].join(', ') %>">...</li>
+          title="<%= html_escape(dojo.tags[5..].join(', ')) %>">...</li>
     <% end %>
   </ul>
 
   <p class="dojo-description">
-    <%= dojo.description %>
+    <%= html_escape(dojo.description) %>
     <% if dojo.is_private %>
       <%= link_to 'Private', doc_path('private-dojo'), class: 'dojo-private' %>
     <% end %>

config/brakeman.ignore

@@ -14,6 +14,7 @@
       get doc_path(param)
       doc      = Document.new(param)
       expected = Kramdown::Document.new(doc.content, input: 'GFM').to_html
+      expected = ApplicationController.helpers.sanitize_content(expected)
       expect(response.body).to include(expected.strip)
     end
 
@@ -23,4 +24,4 @@
       expect(response.status).to eq 302
     end
   end
-end 
+end