CSP: AppendCSPRule adds the sources to an earlier directive when the directive name appears in one of its sources (e.g. https://frame-src.example.com)

Open Beginner friendly
  • Is something wrong?
#5,161 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

A pull request for this has already been merged.

  • #5172 by @RyoKusnadi — merged

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
79/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go
Domain
backend, security

Research direction

Read web/middlewares/secure.go around line 364 and the AppendCSPRule subtests in web/middlewares/secure_test.go. Check how appendCSPRule finds a directive when an earlier source contains its name, then run the focused tests. Done means the matching source is appended only to the named directive, including the reported frame-src and img-src cases.

Written by the indexing model from the issue text.

Description

ai-driven-fuzzing bug Minor

Steps to reproduce

Configuration (cozy.yaml) with a host whose name contains a CSP directive name in an allow list that comes before frame-src in the policy:

csp_allowlist:
  default: https://frame-src.example.com

and an installed app whose intent services are hosted outside the cozy domain (service_url_flag resolving to https://svc.example.com:8443). Open the app (GET https://<slug>.alice.cozy.example/). allowExternalIntentServicesFrame (web/apps/serve.go:191) calls AppendCSPRule(c, "frame-src", "https://svc.example.com:8443").

The policy built by Secure starts with:

default-src 'self' https://cozy.example wss://cozy.example https://frame-src.example.com/;script-src ...;frame-src https://*.cozy.example https://frame-src.example.com/ 'self' ...;

After the append:

default-src 'self' https://cozy.example wss://cozy.example https://frame-src.example.com/ https://svc.example.com:8443;script-src ...;frame-src https://*.cozy.example https://frame-src.example.com/ 'self' ...;

The fuzzer found it with a bare frame-src token in the allow list (url.Parse("frame-src") succeeds, so validCSPList keeps it), which gives the same result.

Unit test (web/middlewares/secure_test.go, in the AppendCSPRule subtest):

r = appendCSPRule("default-src 'self' https://frame-src.example.com/;frame-src 'self';", "frame-src", "https://svc.example.com")
assert.Equal(t, "default-src 'self' https://frame-src.example.com/;frame-src 'self' https://svc.example.com;", r)

r = appendCSPRule("default-src 'self' https://img-src.cdn.example/;img-src data:;", "img-src", "https://idp.example.com")
assert.Equal(t, "default-src 'self' https://img-src.cdn.example/;img-src data: https://idp.example.com;", r)

Fuzzer input (go test fuzz v1 corpus of FuzzCSPAssembly: host, allow list, per-context list, OrgID, OrgDomain, flag, OIDC logo, operations):

go test fuzz v1
string("alice.cozy.example")
string("https://frame-src.example.com")
string("")
string("")
string("")
string("https://svc.example.com:8443")
string("")
[]byte("\x01\x00")

Expected

Only the frame-src directive gets https://svc.example.com:8443; default-src stays as it was.

Observed

directive-name-in-source: appending ["https://svc.example.com:8443"] to frame-src changes
"default-src 'self' https://cozy.example wss://cozy.example https://frame-src.example.com/;...;frame-src https://*.cozy.example https://frame-src.example.com/ 'self' ...;"
into a policy where default-src ends with "https://frame-src.example.com/ https://svc.example.com:8443" and frame-src is unchanged

Possible cause

web/middlewares/secure.go:364: ruleIndex := strings.Index(currentRules, ruleType) searches the whole policy for the name, so it can match inside a source of another directive (https://frame-src.example.com/), or a token equal to the name. The code then treats the text from that point up to the next ; as the directive. Suggested fix: split the policy on ;, and compare the first whitespace-separated token of each directive (case-insensitive) with ruleType, as CSP3 parses a policy. Then append to that directive, or add the directive when no directive matches. This also handles the 'none' case at lines 371–373 on the real directive.

Introduced by

c45f6b91f, 2018-10-17, "Rely on CSP frame-ancestors instead of X-Frame-Options". This commit added appendCSPRule with ruleIndex := strings.Index(currentRules, ruleType), which does not check that the match is a directive name. At that time it was only used for frame-ancestors. Later commits reused it for frame-src, img-src, form-action and default-src, with more configurable sources before them. Confidence: likely.

Impact

To trigger it, a source containing a directive name must come before that directive in the policy. These sources come from the CSP allow lists of the configuration (csp_allowlist, csp_per_context), which an administrator sets. Untrusted users cannot add them. When it happens, it affects every page that appends to that directive on the instance:

  • the appended source is missing from the intended directive. For example, the iframe of an external intent service is blocked (frame-src), the OIDC logo of the sharing discovery page does not load (img-src), or an intent's frame ancestor is refused (frame-ancestors), so the feature stops working;
  • an unrelated directive gets the source, usually default-src, so the policy allows more than intended for the fetch types that fall back to it. An appended 'none' (form-action in web/files/files.go:1252, web/remote/nextcloud.go:185) ends up in the wrong directive, so form-action is not restricted.

No crash or 5xx occurs.

Fuzzing scenario

28-csp-header-assembly (Content-Security-Policy built by Secure, then patched with AppendCSPRule as the handlers do), found after about 59,000 executions (2m9s) and again after about 255,000 executions (11m52s), on commit af571251c.

Found by twake-fuzz (coverage-guided fuzzing), scenario 28-csp-header-assembly, commit af571251c.

Dominant language
Go
Stars
1.3k
Forks
147
Avg merge
22h 40m
Merged PRs (30d)
134

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from linagora/cozy-stack

All issues in linagora/cozy-stack

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.