Skip to content

Commit 4dae857

Browse files
gmcgibbonbyroot
authored andcommitted
Add CSP directive validation
Validate directives to make sure they don't include semicolons or whitespace. These are special and denote lists and termination of directives. [CVE-2024-54133]
1 parent 7bb8fe2 commit 4dae857

File tree

2 files changed

+48
-4
lines changed

2 files changed

+48
-4
lines changed

actionpack/lib/action_dispatch/http/content_security_policy.rb

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ module ActionDispatch # :nodoc:
2626
# policy.report_uri "/csp-violation-report-endpoint"
2727
# end
2828
class ContentSecurityPolicy
29+
class InvalidDirectiveError < StandardError
30+
end
31+
2932
class Middleware
3033
def initialize(app)
3134
@app = app
@@ -320,9 +323,9 @@ def build_directives(context, nonce, nonce_directives)
320323
@directives.map do |directive, sources|
321324
if sources.is_a?(Array)
322325
if nonce && nonce_directive?(directive, nonce_directives)
323-
"#{directive} #{build_directive(sources, context).join(' ')} 'nonce-#{nonce}'"
326+
"#{directive} #{build_directive(directive, sources, context).join(' ')} 'nonce-#{nonce}'"
324327
else
325-
"#{directive} #{build_directive(sources, context).join(' ')}"
328+
"#{directive} #{build_directive(directive, sources, context).join(' ')}"
326329
end
327330
elsif sources
328331
directive
@@ -332,8 +335,22 @@ def build_directives(context, nonce, nonce_directives)
332335
end
333336
end
334337

335-
def build_directive(sources, context)
336-
sources.map { |source| resolve_source(source, context) }
338+
def validate(directive, sources)
339+
sources.flatten.each do |source|
340+
if source.include?(";") || source != source.gsub(/[[:space:]]/, "")
341+
raise InvalidDirectiveError, <<~MSG.squish
342+
Invalid Content Security Policy #{directive}: "#{source}".
343+
Directive values must not contain whitespace or semicolons.
344+
Please use multiple arguments or other directive methods instead.
345+
MSG
346+
end
347+
end
348+
end
349+
350+
def build_directive(directive, sources, context)
351+
resolved_sources = sources.map { |source| resolve_source(source, context) }
352+
353+
validate(directive, resolved_sources)
337354
end
338355

339356
def resolve_source(source, context)

actionpack/test/dispatch/content_security_policy_test.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,33 @@ def test_dup
2323
assert_equal copied.build, @policy.build
2424
end
2525

26+
def test_whitespace_validation
27+
@policy.base_uri "https://some.url https://other.url"
28+
29+
error = assert_raises(ActionDispatch::ContentSecurityPolicy::InvalidDirectiveError) do
30+
@policy.build
31+
end
32+
assert_equal(<<~MSG.squish, error.message)
33+
Invalid Content Security Policy base-uri: "https://some.url https://other.url".
34+
Directive values must not contain whitespace or semicolons.
35+
Please use multiple arguments or other directive methods instead.
36+
MSG
37+
end
38+
39+
40+
def test_semicolon_validation
41+
@policy.base_uri "https://some.url; script-src https://other.url"
42+
43+
error = assert_raises(ActionDispatch::ContentSecurityPolicy::InvalidDirectiveError) do
44+
@policy.build
45+
end
46+
assert_equal(<<~MSG.squish, error.message)
47+
Invalid Content Security Policy base-uri: "https://some.url; script-src https://other.url".
48+
Directive values must not contain whitespace or semicolons.
49+
Please use multiple arguments or other directive methods instead.
50+
MSG
51+
end
52+
2653
def test_mappings
2754
@policy.script_src :data
2855
assert_equal "script-src data:", @policy.build

0 commit comments

Comments
 (0)