Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/secure_headers/headers/content_security_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def minify_source_list(directive, source_list)
else
source_list = populate_nonces(directive, source_list)
source_list = reject_all_values_if_none(source_list)
source_list = normalize_uri_paths(source_list)

unless directive == REPORT_URI || @preserve_schemes
source_list = strip_source_schemes(source_list)
Expand All @@ -147,6 +148,19 @@ def reject_all_values_if_none(source_list)
end
end

def normalize_uri_paths(source_list)
source_list.map do |source|
# Normalize domains ending in a single / as without omitting the slash accomplisheg the same.
# https://www.w3.org/TR/CSP3/#match-paths § 6.6.2.10 Step 2
if source.chomp("/").include?("/")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing this might not be normalizing is a CSP directive like https://www.example.com/ since the protocol includes /.

The quoted spec is about matching on the path-part, not the entire URL that is in the directive.

It would be good to address this, with unit tests as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a test and some code to fix this. The code doesn't feel great, I'd welcome more elegant solutions here!

source
else
source.chomp("/")
end
end
end


# Removes duplicates and sources that already match an existing wild card.
#
# e.g. *.github.com asdf.github.com becomes *.github.com
Expand Down
10 changes: 9 additions & 1 deletion spec/lib/secure_headers/headers/content_security_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,17 @@ module SecureHeaders
expect(csp.value).to eq("default-src * 'unsafe-inline' 'unsafe-eval' data: blob:")
end

it "normalizes source expressions that end with a trailing /" do
config = {
default_src: %w(a.example.org/ b.example.com/ c.example.net/foo/ b.example.co/bar)
}
csp = ContentSecurityPolicy.new(config)
expect(csp.value).to eq("default-src a.example.org b.example.com c.example.net/foo/ b.example.co/bar")
end

it "minifies source expressions based on overlapping wildcards" do
config = {
default_src: %w(a.example.org b.example.org *.example.org https://*.example.org)
default_src: %w(a.example.org b.example.org *.example.org https://*.example.org c.example.org/)
}
csp = ContentSecurityPolicy.new(config)
expect(csp.value).to eq("default-src *.example.org")
Expand Down