fix: correctly encode user entered data in attributes, Javascript, an…

…d HTML

Fixes: #325

lib/pact_broker/api/renderers/html_pact_renderer.rb

@@ -4,6 +4,7 @@
 require 'pact/doc/markdown/consumer_contract_renderer'
 require 'pact_broker/api/pact_broker_urls'
 require 'pact_broker/logging'
+require 'rack'
 
 module PactBroker
   module Api
@@ -58,8 +59,8 @@ def pact_metadata
               #{badge_list_item}
               #{badge_markdown_item}
               <li>
-                <span class='name'>#{@pact.consumer.name} version:</span>
-                <span class='value'>#{@pact.consumer_version_number}#{tags}</span>
+                <span class='name'>#{consumer_name} version:</span>
+                <span class='value'>#{consumer_version_number}#{tags}</span>
               </li>
               <li>
                 <span class='name' title='#{published_date}'>Date published:</span>
@@ -75,9 +76,9 @@ def pact_metadata
                 <a href=\"#{base_url}/\">Home</a>
               </li>
               <li>
-                <span data-consumer-name=\"#{@pact.consumer.name}\"
-                      data-provider-name=\"#{@pact.provider.name}\"
-                      data-consumer-version-number=\"#{@pact.consumer_version_number}\"
+                <span data-consumer-name=\"#{consumer_name}\"
+                      data-provider-name=\"#{provider_name}\"
+                      data-consumer-version-number=\"#{consumer_version_number}\"
                       data-pact-url=\"#{pact_url}\"
                       class='more-options glyphicon glyphicon-option-horizontal'
                       aria-hidden='true'></span>
@@ -88,7 +89,7 @@ def pact_metadata
 
         def badge_list_item
             "<li class='pact-badge'>
-              <img src='#{badge_url}'/>
+              <img src=\"#{badge_url}\"/>
             </li>
             "
         end
@@ -117,7 +118,19 @@ def base_url
         end
 
         def title
-          "Pact between #{@pact.consumer.name} and #{@pact.provider.name}"
+          "Pact between #{consumer_name} and #{provider_name}"
+        end
+
+        def consumer_version_number
+          h(@pact.consumer_version_number)
+        end
+
+        def consumer_name
+          h(@pact.consumer.name)
+        end
+
+        def provider_name
+          h(@pact.provider.name)
         end
 
         def published_date
@@ -154,7 +167,8 @@ def badge_url
 
         def tags
           if @pact.consumer_version_tag_names.any?
-            " (#{@pact.consumer_version_tag_names.join(", ")})"
+            tag_names = @pact.consumer_version_tag_names.collect{ |t| h(t) }.join(", ")
+            " (#{tag_names})"
           else
             ""
           end
@@ -179,6 +193,10 @@ def consumer_contract
           logger.info "Could not parse the following content to a Pact due to #{e.class} #{e.message}, showing raw content instead: #{@json_content}"
           raise NotAPactError
         end
+
+        def h string
+          Rack::Utils.escape_html(string)
+        end
       end
     end
   end

lib/pact_broker/badges/service.rb

@@ -4,6 +4,7 @@
 require 'pact_broker/logging'
 require 'pact_broker/configuration'
 require 'pact_broker/build_http_options'
+require 'erb'
 
 module PactBroker
   module Badges
@@ -45,7 +46,7 @@ def badge_title pact, label, initials, metadata
         title = case (label || '').downcase
           when 'consumer' then consumer_name
           when 'provider' then provider_name
-          else "#{consumer_name}%2F#{provider_name}"
+          else "#{consumer_name}/#{provider_name}"
         end
         "#{title} pact".downcase
       end
@@ -111,7 +112,7 @@ def build_shield_io_uri left_text, right_text, color
       end
 
       def escape_text text
-        text.gsub(" ", "%20").gsub("-", "--").gsub("_", "__")
+        ERB::Util.url_encode(text.gsub("-", "--").gsub("_", "__"))
       end
 
       def do_request(uri)

lib/pact_broker/doc/views/layouts/main.haml

@@ -2,4 +2,4 @@
   %head
     %link{rel: 'stylesheet', href: "#{base_url}/css/bootstrap.min.css"}
   %body{style: 'margin:20px'}
-    = yield
+    != yield

lib/pact_broker/test/test_data_builder.rb

@@ -73,14 +73,14 @@ def create_condor
         self
       end
 
-      def create_pact_with_hierarchy consumer_name = "Consumer", consumer_version_number = "1.2.3", provider_name = "Provider", json_content = default_json_content
+      def create_pact_with_hierarchy consumer_name = "Consumer", consumer_version_number = "1.2.3", provider_name = "Provider", json_content = nil
         use_consumer(consumer_name)
         create_consumer(consumer_name) if !consumer
         use_provider(provider_name)
         create_provider provider_name if !provider
         use_consumer_version(consumer_version_number)
         create_consumer_version(consumer_version_number) if !consumer_version
-        create_pact json_content: json_content
+        create_pact json_content: json_content || default_json_content
         self
       end
 
@@ -436,10 +436,10 @@ def set_created_at_if_set created_at, table_name, selector
       def default_json_content
         {
           "consumer" => {
-             "name" => "Condor"
+             "name" => consumer.name
            },
            "provider" => {
-             "name" => "Pricing Service"
+             "name" => provider.name
            },
            "interactions" => [],
            "random" => rand

lib/pact_broker/ui/controllers/clusters.rb

@@ -19,7 +19,7 @@ def initialize pacticipants, relationships
 
         get "/" do
           view_model = ViewDomain::IndexItems.new(pacticipant_service.find_index_items, base_url: base_url)
-          haml 'clusters/show', locals: {relationships: view_model, base_url: base_url}
+          haml 'clusters/show', locals: { relationships: view_model, base_url: base_url }, escape_html: true
         end
 
       end

lib/pact_broker/ui/controllers/groups.rb

@@ -13,13 +13,13 @@ class Groups < Base
           pacticipant = pacticipant_service.find_pacticipant_by_name(params[:name])
           erb :'groups/show.html', {
               locals: {
-                csv_path: "#{base_url}/groups/#{params[:name]}.csv",
+                csv_path: "#{base_url}/groups/#{ERB::Util.url_encode(params[:name])}.csv",
                 pacticipant_name: params[:name],
                 repository_url: pacticipant&.repository_url,
                 base_url: base_url
               }
             }, {
-              layout: 'layouts/main'
+              layout: 'layouts/main',
             }
         end
 

lib/pact_broker/ui/controllers/index.rb

@@ -36,7 +36,7 @@ class Index < Base
             base_url: base_url
           }
 
-          haml page, {locals: locals, layout: :'layouts/main'}
+          haml page, { locals: locals, layout: :'layouts/main', escape_html: true }
         end
 
         def set_headers

lib/pact_broker/ui/controllers/matrix.rb

@@ -42,7 +42,7 @@ class Matrix < Base
             Padrino.logger.exception(e) unless e.is_a?(PactBroker::Error)
             locals[:errors] = [e.message]
           end
-          haml :'matrix/show', {locals: locals, layout: :'layouts/main'}
+          haml :'matrix/show', { locals: locals, layout: :'layouts/main', escape_html: true }
         end
 
         get "/provider/:provider_name/consumer/:consumer_name" do
@@ -60,7 +60,7 @@ class Matrix < Base
             badge_url: nil,
             base_url: base_url
           }
-          haml :'matrix/show', {locals: locals, layout: :'layouts/main'}
+          haml :'matrix/show', { locals: locals, layout: :'layouts/main', escape_html: true }
         end
 
         def create_selector_objects(selector_hashes)

lib/pact_broker/ui/views/groups/show.html.erb

@@ -31,13 +31,13 @@ body{
 <!-- developed by Duncan Alexander - hypothete.com -->
 </head>
 <body>
-<h1>Network graph of <%= pacticipant_name %> relationships</h1>
+<h1>Network graph of <%= escape_html(pacticipant_name) %> relationships</h1>
 
 <% if repository_url %>
 <p>Repository URL:
 
 <%
-repository_link = "<a href='#{repository_url}'>#{escape_html(repository_url)}</a>"
+repository_link = "<a href=\"#{repository_url}\">#{repository_url}</a>"
 %>
 
 <%= Sanitize.fragment(repository_link, Sanitize::Config::BASIC) %>
@@ -105,7 +105,7 @@ var relationshipPath = function(relationship, nodeLocations, pacticipants) {
 
 var latestPactUrl = function (consumerName, providerName) {
     //TODO send this with the relationship data
-    return '<%= base_url %>/pacts/provider/' + providerName + '/consumer/' + consumerName + '/latest';
+    return '<%= base_url %>' + '/pacts/provider/' + encodeURIComponent(providerName) + '/consumer/' + encodeURIComponent(consumerName) + '/latest';
 };
 
 var relationshipData = function(pacticipant, relationships) {

lib/pact_broker/ui/views/index/show-with-tags.haml

@@ -1,9 +1,9 @@
 %body
-  = render :haml, :'index/_css_and_js', :layout => false
+  != render :haml, :'index/_css_and_js', :layout => false
   .container
-    = render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: false, base_url: base_url}
+    != render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: false, base_url: base_url}
     - if index_items.empty?
-      = render :haml, :'index/_getting-started', :layout => false
+      != render :haml, :'index/_getting-started', :layout => false
     %h1.page-header
       Pacts
     %table.table.table-bordered.table-striped{ id: 'relationships' }
@@ -37,10 +37,10 @@
           %tr{'data-pact-versions-url': index_item.pact_versions_url, 'data-consumer-name': index_item.consumer_name, 'data-provider-name': index_item.provider_name, 'data-integration-url': index_item.integration_url}
             %td.consumer
               %a{:href => index_item.consumer_group_url }
-                = escape_html(index_item.consumer_name)
+                = index_item.consumer_name
             %td.consumer-version-number{"data-text": index_item.consumer_version_order}
               %div.clippable
-                = escape_html(index_item.consumer_version_number)
+                = index_item.consumer_version_number
                 - if index_item.consumer_version_number
                   %button.clippy.invisible{ title: "Copy to clipboard" }
                     %span.glyphicon.glyphicon-copy
@@ -49,24 +49,24 @@
                   latest
               - index_item.consumer_version_latest_tag_names.each do | tag_name |
                 .tag.label.label-primary
-                  = escape_html(tag_name)
+                  = tag_name
             %td.pact
               %span.pact
                 %a{ href: index_item.pact_url, title: "View pact" }
               %span.pact-matrix
                 %a{ href: index_item.pact_matrix_url, title: "View pact matrix" }
             %td.provider
               %a{ href: index_item.provider_group_url }
-                = escape_html(index_item.provider_name)
+                = index_item.provider_name
             %td.provider-version-number
               %div.clippable
-                = escape_html(index_item.provider_version_number)
+                = index_item.provider_version_number
                 - if index_item.provider_version_number
                   %button.clippy.invisible{ title: "Copy to clipboard" }
                     %span.glyphicon.glyphicon-copy
               - index_item.provider_version_latest_tag_names.each do | tag_name |
                 .tag.label.label-primary
-                  = escape_html(tag_name)
+                  = tag_name
             %td{"data-text": index_item.publication_date_of_latest_pact_order}
               = index_item.publication_date_of_latest_pact.gsub("about ", "")
             %td{ class: index_item.webhook_status }
@@ -86,7 +86,7 @@
     %div.pagination
 
     - pagination_locals = { page_number: page_number, page_size: page_size, pagination_record_count: pagination_record_count, current_page_size: current_page_size }
-    = render :haml, :'index/_pagination', :layout => false, locals: pagination_locals
+    != render :haml, :'index/_pagination', :layout => false, locals: pagination_locals
 
   :javascript
     $(function(){

lib/pact_broker/ui/views/index/show.haml

@@ -1,9 +1,9 @@
 %body
-  = render :haml, :'index/_css_and_js', :layout => false
+  != render :haml, :'index/_css_and_js', :layout => false
   .container
-    = render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: true, base_url: base_url}
+    != render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: true, base_url: base_url}
     - if index_items.empty?
-      = render :haml, :'index/_getting-started', :layout => false
+      != render :haml, :'index/_getting-started', :layout => false
     %h1.page-header
       Pacts
     %table.table.table-bordered.table-striped{ id: 'relationships' }
@@ -32,15 +32,15 @@
             %td
             %td.consumer
               %a{ href: index_item.consumer_group_url }
-                = escape_html(index_item.consumer_name)
+                = index_item.consumer_name
             %td.pact
               %span.pact
                 %a{ href: index_item.latest_pact_url, :title => "View pact" }
               %span.pact-matrix
                 %a{ href: index_item.pact_matrix_url, title: "View pact matrix" }
             %td.provider
               %a{ href: index_item.provider_group_url }
-                = escape_html(index_item.provider_name)
+                = index_item.provider_name
             %td
             %td{"data-text": index_item.publication_date_of_latest_pact_order}
               = index_item.publication_date_of_latest_pact
@@ -58,7 +58,7 @@
     %div.pagination
 
     - pagination_locals = { page_number: page_number, page_size: page_size, pagination_record_count: pagination_record_count, current_page_size: current_page_size }
-    = render :haml, :'index/_pagination', :layout => false, locals: pagination_locals
+    != render :haml, :'index/_pagination', :layout => false, locals: pagination_locals
 
 
   :javascript

lib/pact_broker/ui/views/layouts/main.haml

@@ -6,4 +6,4 @@
     %link{rel: 'stylesheet', href: "#{base_url}/css/bootstrap.min.css"}
     %script{type: 'text/javascript', src:"#{base_url}/javascripts/jquery-3.3.1.min.js"}
     %script{type: 'text/javascript', src:"#{base_url}/js/bootstrap.min.js"}
-  = yield
+  != yield

lib/pact_broker/ui/views/matrix/show.haml

@@ -20,15 +20,14 @@
     - if defined?(errors) && errors.any?
       - errors.each do | error |
         %div.alert.alert-danger
-          = escape_html(error)
-
+          = error
 
     %form{action: "#{base_url}/matrix", onsubmit:'return onSubmit()'}
       - selectors.each_with_index do | selector, index |
         .selector
           %label{for: "pacticipant#{index}"}
             Pacticipant name
-          %input{name: 'q[]pacticipant', id: "pacticipant1#{index}", class: 'pacticipant_name', value: escape_html(selector.pacticipant_name)}
+          %input{name: 'q[]pacticipant', id: "pacticipant1#{index}", class: 'pacticipant_name', value: selector.pacticipant_name}
 
           .input-group
 
@@ -45,9 +44,9 @@
               %option{ value: 'specify-all-tagged', selected: selector.specify_all_tagged }
                 All versions with tag...
 
-            %input{name: 'q[]version', type: 'text', id: "pacticipant#{index}_version", class: 'version', value: escape_html(selector.pacticipant_version_number)}
+            %input{name: 'q[]version', type: 'text', id: "pacticipant#{index}_version", class: 'version', value: selector.pacticipant_version_number}
 
-            %input{name: 'q[]tag', type: 'text', id: "pacticipant#{index}_tag", class: 'tag', value: escape_html(selector.tag)}
+            %input{name: 'q[]tag', type: 'text', id: "pacticipant#{index}_tag", class: 'tag', value: selector.tag}
 
             %input{name: 'q[]latest', value: 'true', hidden: true, class: 'latest-flag'}
 

public/javascripts/pact.js

@@ -39,10 +39,14 @@ $(document).ready(function() {
     });
 });
 
+function h(string) {
+  return jQuery('<div/>').text(string).html();
+}
+
 function createDeletionConfirmationText(data) {
   return `Do you wish to delete the pact for version ${
-    data.consumerVersionNumber
-  } of ${data.consumerName}?`;
+    h(data.consumerVersionNumber)
+  } of ${h(data.consumerName)}?`;
 }
 
 function confirmDeleteResource(

script/seed.rb

@@ -58,13 +58,11 @@
     url: "http://localhost:9292/verification-published-webhook",
     body: webhook_body.to_json)
   .set_now(Date.today - 101)
-  .tap{ | it |
-    2.times do | i |
-      it.create_pact_with_verification("Foo", i.to_s, "Bar", i.to_s)
-      .create_pact_with_verification("Bar", i.to_s, "Foo", i.to_s)
-      .add_day
-    end
-  }.create_pact_with_hierarchy("Foo", "100", "Bar")
+  .create_pact_with_hierarchy("Foo/Foo", "100", "Bar/Bar")
+  .create_pact_with_hierarchy("Foo", "1", "Bar")
+  .create_pact_with_hierarchy("<script>alert('hello')</script>", "<script>alert(\"version\")</script>", "Bar/Bar")
+  .create_consumer_version_tag("prod")
+  .create_verification(provider_version: "1", tag_names: "prod")
 
 
   # .create_certificate(path: 'spec/fixtures/certificates/self-signed.badssl.com.pem')