codeql: close java/unvalidated-url-redirection #596-#601 (Task 8) - #46
Merged
Conversation
Real fixes (CodeQL-modeled) for 6 alerts + new shared validator:
1. New PSRedirectValidation helper in perc-security-utils - port of
004 T051 (PR #1344): validateRedirectUrl (whitelist),
validateInternalRedirectUrl (path-only), createDefaultWhitelist.
JUnit 4 port of 004 test suite (33 tests).
2. PSSecurityFilter (#599 #600 #601): HTTPS-upgrade + form-auth
redirects route through sendValidatedRedirect - relative paths
internal-only; absolute URLs validated against publicCmsHostname
whitelist (fallback server name after isValidHostHeader). HTTPS
upgrade prefers configured publicCmsHostname over Host header.
sanitizeForLog strips control chars.
3. PSCommentsRestService (#596 #597): seeOther from Referer reduced to
same-document relative path via toRelativeRedirectTarget +
validateInternalRedirectUrl; invalid targets -> 204. sanitizeForLog
added. comments pom gains perc-security-utils dep.
4. PSUncaughtError (#598): error redirect no longer rebuilt from
Referer host; always local {contextPath}/error.html validated.
Verification: compile + tests pass (109 perc-security-utils tests incl.
33 PSRedirectValidationTest; comments/common BUILD SUCCESS).
# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# interactive rebase in progress; onto 76f4ae3
# Last command done (1 command done):
# pick dae8bebcab codeql: close java/unvalidated-url-redirection #596-#601 (Task 8)
# No commands remaining.
# You are currently rebasing branch 'codeql/url-redirection' on '76f4ae3e0f'.
#
# Changes to be committed:
# modified: CHANGELOG.md
# modified: deliverytiersuite/delivery-tier-suite/comments/pom.xml
# modified: deliverytiersuite/delivery-tier-suite/comments/src/main/java/com/percussion/delivery/comments/services/PSCommentsRestService.java
# modified: deliverytiersuite/delivery-tier-suite/common/src/main/java/com/percussion/delivery/exceptions/PSUncaughtError.java
# new file: modules/perc-security-utils/src/main/java/com/percussion/security/utils/PSRedirectValidation.java
# new file: modules/perc-security-utils/src/test/java/com/percussion/security/utils/PSRedirectValidationTest.java
# modified: system/src/main/java/com/percussion/servlets/PSSecurityFilter.java
#
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Address CodeQL review comments on PR #46 (#701/#702): reconstruct seeOther/sendRedirect targets from validated URI components and document the GHAS-unmodelable residual. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
4 tasks
Collaborator
Author
|
Superseded by #50 (consolidated onto current |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes 6 CodeQL
java/unvalidated-url-redirectionalerts (#596-#601) with real fixes (CodeQL-modeled — no paths-ignore, no sink-line suppressions).New shared validator: PSRedirectValidation
Port of the 004 T051 validator (PR #1344) into
modules/perc-security-utils:validateRedirectUrl(url, allowedDomains)— whitelist-based (rejects protocol-relative, data:, javascript:, traversal, non-whitelisted hosts)validateInternalRedirectUrl(url)— path-only (rejects all absolute URLs)createDefaultWhitelist(domain)— domain + www variantJUnit 4 port of the 004 test suite — 33 tests, all pass.
Sinks fixed
PSSecurityFilter.handleFormAuthsendValidatedRedirect— relative paths internal-only; absolute URLs validated againstpublicCmsHostnamewhitelist (fallback: server name afterisValidHostHeader)PSSecurityFilterHTTPS upgradepublicCmsHostnameover Host header; validated redirectPSCommentsRestServiceseeOtherfrom Referer reduced to same-document relative path (toRelativeRedirectTarget) +validateInternalRedirectUrl; invalid targets → 204PSUncaughtError{contextPath}/error.htmlvalidatedAlso adds
sanitizeForLog(control-char strip) to PSSecurityFilter and PSCommentsRestService for log-injection hygiene, and theperc-security-utilsdependency to the comments module pom (same convention as common module).Verification
Notes
PSRedirectValidationadapted to the javax stack (004 uses jakarta) and JUnit 4 (module has no JUnit 5).perc-security-utilsdependency.Version.propertieswas not modified.