Skip to content

Commit cbe7014

Browse files
committed
prevent the ability to muck with original CSP settings when using a dynamic policy
/cc @ptoomey3
1 parent 8a314e1 commit cbe7014

File tree

4 files changed

+32
-5
lines changed

4 files changed

+32
-5
lines changed

lib/secure_headers.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class << self
4949
def override_content_security_policy_directives(request, additions)
5050
config = config_for(request)
5151
if config.current_csp == OPT_OUT
52-
config.csp = config.dynamic_csp = {}
52+
config.dynamic_csp = {}
5353
end
5454

5555
config.dynamic_csp = config.current_csp.merge(additions)

lib/secure_headers/configuration.rb

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ class Configuration
33
DEFAULT_CONFIG = :default
44
NOOP_CONFIGURATION = "secure_headers_noop_config"
55
class NotYetConfiguredError < StandardError; end
6+
class IllegalPolicyModificationError < StandardError; end
67
class << self
78
# Public: Set the global default configuration.
89
#
@@ -23,12 +24,12 @@ def default(&block)
2324
# if no value is supplied.
2425
#
2526
# Returns: the newly created config
26-
def override(name, base = DEFAULT_CONFIG)
27+
def override(name, base = DEFAULT_CONFIG, &block)
2728
unless get(base)
2829
raise NotYetConfiguredError, "#{base} policy not yet supplied"
2930
end
3031
override = @configurations[base].dup
31-
yield(override)
32+
override.instance_eval &block if block_given?
3233
add_configuration(name, override)
3334
end
3435

@@ -99,7 +100,7 @@ def deep_copy_if_hash(value)
99100
end
100101

101102
attr_writer :hsts, :x_frame_options, :x_content_type_options,
102-
:x_xss_protection, :csp, :x_download_options, :x_permitted_cross_domain_policies,
103+
:x_xss_protection, :x_download_options, :x_permitted_cross_domain_policies,
103104
:hpkp, :dynamic_csp, :secure_cookies
104105

105106
attr_reader :cached_headers, :csp, :dynamic_csp, :secure_cookies
@@ -166,6 +167,14 @@ def validate_config!
166167

167168
protected
168169

170+
def csp=(new_csp)
171+
if self.dynamic_csp
172+
raise IllegalPolicyModificationError, "You are attempting to modify CSP settings directly. Use dynamic_csp= isntead."
173+
end
174+
175+
@csp = new_csp
176+
end
177+
169178
def cached_headers=(headers)
170179
@cached_headers = headers
171180
end

spec/lib/secure_headers/configuration_spec.rb

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

3737
config = Configuration.get(:test_override)
3838
noop = Configuration.get(Configuration::NOOP_CONFIGURATION)
39-
[:csp, :dynamic_csp].each do |key|
39+
[:csp, :dynamic_csp, :secure_cookies].each do |key|
4040
expect(config.send(key)).to eq(noop.send(key)), "Value not copied: #{key}."
4141
end
4242
end

spec/lib/secure_headers_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,24 @@ module SecureHeaders
129129
SecureHeaders.header_hash_for(request)
130130
end
131131

132+
it "doesn't allow you to muck with csp configs when a dynamic policy is in use" do
133+
default_config = Configuration.default
134+
expect { default_config.csp = {} }.to raise_error(NoMethodError)
135+
136+
# config is frozen
137+
expect { default_config.send(:csp=, {}) }.to raise_error(RuntimeError)
138+
139+
SecureHeaders.append_content_security_policy_directives(request, script_src: %w(anothercdn.com))
140+
new_config = SecureHeaders.config_for(request)
141+
expect { new_config.send(:csp=, {}) }.to raise_error(Configuration::IllegalPolicyModificationError)
142+
143+
expect do
144+
new_config.instance_eval do
145+
new_config.csp = {}
146+
end
147+
end.to raise_error(Configuration::IllegalPolicyModificationError)
148+
end
149+
132150
it "overrides individual directives" do
133151
Configuration.default do |config|
134152
config.csp = {

0 commit comments

Comments
 (0)