Merge pull request #419 from twitter/escape-semi-colons

Escape semi colons in directive source lists

Author: oreoshake

lib/secure_headers/configuration.rb

@@ -126,7 +126,9 @@ def deep_copy_if_hash(value)
     # The list of attributes that must respond to a `make_header` method
     HEADERABLE_ATTRIBUTES = (CONFIG_ATTRIBUTES - [:cookies]).freeze
 
-    attr_accessor(*CONFIG_ATTRIBUTES_TO_HEADER_CLASSES.keys)
+    attr_writer(*(CONFIG_ATTRIBUTES_TO_HEADER_CLASSES.reject { |key| [:csp, :csp_report_only].include?(key) }.keys))
+
+    attr_reader(*(CONFIG_ATTRIBUTES_TO_HEADER_CLASSES.keys))
 
     @script_hashes = nil
     @style_hashes = nil

lib/secure_headers/headers/content_security_policy.rb

@@ -103,10 +103,15 @@ def build_media_type_list_directive(directive)
     # Returns a string representing a directive.
     def build_source_list_directive(directive)
       source_list = @config.directive_value(directive)
-
       if source_list != OPT_OUT && source_list && source_list.any?
-        normalized_source_list = minify_source_list(directive, source_list)
-        [symbol_to_hyphen_case(directive), normalized_source_list].join(" ")
+        minified_source_list = minify_source_list(directive, source_list).join(" ")
+
+        if minified_source_list.include?(";")
+          Kernel.warn("#{directive} contains a ; in '#{minified_source_list}' which will raise an error in future versions. It has been replaced with a blank space.")
+        end
+
+        escaped_source_list = minified_source_list.gsub(";", " ")
+        [symbol_to_hyphen_case(directive), escaped_source_list].join(" ").strip
       end
     end
 

spec/lib/secure_headers/headers/content_security_policy_spec.rb

@@ -28,6 +28,11 @@ module SecureHeaders
         expect(ContentSecurityPolicy.new.value).to eq("default-src https:; form-action 'self'; img-src https: data: 'self'; object-src 'none'; script-src https:; style-src 'self' 'unsafe-inline' https:")
       end
 
+      it "deprecates and escapes semicolons in directive source lists" do
+        expect(Kernel).to receive(:warn).with("frame_ancestors contains a ; in 'google.com;script-src *;.;' which will raise an error in future versions. It has been replaced with a blank space.")
+        expect(ContentSecurityPolicy.new(frame_ancestors: %w(https://google.com;script-src https://*;.;)).value).to eq("frame-ancestors google.com script-src * .")
+      end
+
       it "discards 'none' values if any other source expressions are present" do
         csp = ContentSecurityPolicy.new(default_opts.merge(child_src: %w('self' 'none')))
         expect(csp.value).not_to include("'none'")