Skip to content

fix(security): backport SQL injection + auth fixes to 26.04.28 (hotfix)#35608

Closed
sfreudenthaler wants to merge 11 commits into
release-26.04.28-02from
release-26.04.28-03
Closed

fix(security): backport SQL injection + auth fixes to 26.04.28 (hotfix)#35608
sfreudenthaler wants to merge 11 commits into
release-26.04.28-02from
release-26.04.28-03

Conversation

@sfreudenthaler

Copy link
Copy Markdown
Member

Summary

Security hotfix for customers running v26.04.28-02. Cherry-picks 9 commits from PR #35553 onto the 26.04.28 release line — no other changes from main are included.

Do not merge this PR. It exists to run CI and collect reviews. The release (26.04.28-03) is being cut directly from this branch.

What's included

  • SQL injection fixPublishAuditAPIImpl.getPublishAuditStatuses() now uses bound parameters instead of string-concatenated IN (...) clause (Refs: dotCMS/private-issues#581)
  • Auth gate/api/auditPublishing/get and /getAll now require an authenticated backend user with publishing-queue portlet permission (previously unauthenticated)
  • PP authorization — switched from WebResource.InitBuilder to PP auth in AuditPublishingResource
  • Auth header — adds PP auth header to audit poll; prevents 500 on missing token
  • Regression tests and Postman collection updates

Cherry-picked from

PR #35553 (merged to main on 2026-05-06)

Commits cherry-picked (merge-from-main commits excluded):

  • dc515d99 fix(publisher): parameterize getPublishAuditStatuses bundle-id query
  • f783f9dd test(publisher): regression tests for parameterization
  • f68ff49f Apply suggestions from code review
  • 947fda39 fix: remove duplicate placeholders block
  • 10dcb623 fix(rest): require backend user + publishing-queue portlet on AuditPublishingResource
  • 15b6b8c3 test: use dotCMS-specific SecurityException
  • 381d83df add AuditPublishingResourceTest to MainSuite2a
  • 13f70a5e use PP authorization in AuditPublishingResource
  • 071f4da8 fix(publisher): add PP auth header to audit poll and prevent 500 on missing token

mbiuki and others added 9 commits May 7, 2026 09:55
Replaces the manually quoted IN-clause with bound parameters so caller-supplied
bundle ids cannot break out of the SQL literal. Also guards against null/empty
input (which previously NPE'd or produced invalid `IN ()` SQL), and corrects
the catch-block logger to use this class and ERROR level (matching the sibling
deletePublishAuditStatus method).

Refs: dotCMS/private-issues#581
…erization

Adds four integration tests for getPublishAuditStatuses(List<String>):
- happy path returns the requested rows and excludes others
- empty list returns an empty result without producing invalid `IN ()` SQL
- null list returns an empty result without NPE
- bundle ids containing SQL meta-characters (single quotes, OR 1=1,
  '; DROP TABLE; --, comment terminators, escaped quotes) are bound as
  parameters and produce no rows; the audit table remains intact

Refs: dotCMS/private-issues#581
Co-authored-by: semgrep-code-dotcms-test[bot] <183154938+semgrep-code-dotcms-test[bot]@users.noreply.github.com>
…pply

The "Apply suggestion" action on the Semgrep bot's review (commit f68ff49)
appended the bot's suggested code on top of the existing parameterized fix
instead of replacing it, leaving two `placeholders` declarations in the same
method (compile error: "variable placeholders is already defined").

The bot's suggestion was functionally identical to the committed code; the
existing parameterized fix is correct. Removing the duplicate block.
…blishingResource

Adds WebResource.InitBuilder authentication checks to GET /api/auditPublishing/get
and POST /api/auditPublishing/getAll. Both endpoints previously skipped
WebResource.init entirely, so an unauthenticated caller could enumerate publish
audit status for any bundle id (or list of ids) — the SQL injection in the
underlying DB method was the worst-case shape, but even after parameterization
the data exposure remains until the resource is gated.

Both endpoints now require:
- a backend (CMS) user, not a frontend user
- the `publishing-queue` portlet permission
- rejection when no user is present (no anonymous access)

Addresses review feedback from @wezell.
…eTest

The auth check throws com.dotcms.rest.exception.SecurityException
(WebApplicationException subclass), not java.lang.SecurityException.
@test(expected = ...) defaults to the imported class, so a no-arg import
was matching the wrong type and the test errored even though the auth
worked correctly.
…issing token

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

  • This PR fixes #123 or Fixes: #123

  • This PR closes #123 or Closes: #123

  • This PR resolves #123 or Resolves: #123

  • Other supported keywords: fix, fixed, close, closed, resolve, resolved
    Option 2: Link via GitHub UI (Note: won't clear the failed check)

  1. Go to the PR → Development section (right sidebar)

  2. Click "Link issue" and select an existing issue

  3. Push a new commit or re-run the workflow to clear the failed check
    Option 3: Use branch naming
    Create a new branch with one of these patterns:

  • 123-feature-description (number at start)

  • issue-123-feature-description (issue-number at start)

  • feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.---

This comment was automatically generated by the issue linking workflow

@alwaysmeticulous

alwaysmeticulous Bot commented May 7, 2026

Copy link
Copy Markdown

🤖 No test run has been triggered as your Meticulous project has been deactivated (since you haven't viewed any test results in a while). Click here to reactivate.

Last updated for commit d408d3e. This comment will update as new commits are pushed.

Backports the LTS workflow fix from main (PR #35612) to enable manual
triggering with a version override and dynamic version resolution from
.mvn/maven.config.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sfreudenthaler sfreudenthaler requested a review from a team as a code owner May 7, 2026 23:59
@sfreudenthaler

Copy link
Copy Markdown
Member Author

no need to merge. just did this pr to test and for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants