From f88b190ec1c4f4cdc734b133bed13aef0faeab6e Mon Sep 17 00:00:00 2001 From: Nate Chadwick <263952448+natechadwick-intsof@users.noreply.github.com> Date: Sat, 15 Aug 2026 18:34:50 -0400 Subject: [PATCH] codeql: close delivery-view residuals #705-#715 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 11 post-merge residual alerts from PRs #57 and #60 closed: Code fixes (3 alerts): - PercBlogPostView.js (#707, #708): added percSafeUrl() helper (mirror of PercArchiveListView.js) and wrapped both .attr(href) calls. The encodeURIComponent call was already in place from PR #60; the scheme-block is the missing defense. - PercTagListView.js (#715): same pattern as BlogPost — added percSafeUrl() helper, wrapped the .attr(href) call at line 122 (was 115 in GHAS, +7 lines from the added helper). - cui/widgets/app/app.viewmodel.js (#705, #706): the /g flag added in PR #57 fixed the 'first occurrence' CodeQL alert but surfaced a new 'does not escape backslash' alert. Tightened the regex to /[\[\]\]/g so escaping covers [, ], and \ in one global replace. Sink-line suppressions (6 alerts — false positives, PR #60 already added the percSafeUrl() wrapper but GHAS does not model the in-repo helper as a sanitizer barrier): - PercArchiveListView.js:137, 197, 285 (#709, #710, #711) - PercCategoryListView.js:198 (#712) - PercRegistrationView.js:82, 270 (#713, #714) Each // codeql[js/xss-through-dom] anchor carries the same justification text referencing the in-repo percSafeUrl() runtime defense; same re-review date 2027-07-31. Verification: - python3 scripts/verify-suppressions.py PASS - node --check on each modified file syntax OK - python3 scripts/verify-triage-inventory.py PASS JDK 1.8.0 compatible. No CHANGELOG.md entry per AGENTS.md. --- cui/widgets/app/app.viewmodel.js | 4 ++-- delivery/common/js/views/PercArchiveListView.js | 3 +++ delivery/common/js/views/PercBlogPostView.js | 15 ++++++++++++--- delivery/common/js/views/PercCategoryListView.js | 1 + delivery/common/js/views/PercRegistrationView.js | 4 +++- delivery/common/js/views/PercTagListView.js | 10 +++++++++- .../tasks/8.1.x-codeql-baseline/suppressions.md | 15 +++++++++++++-- 7 files changed, 43 insertions(+), 9 deletions(-) diff --git a/cui/widgets/app/app.viewmodel.js b/cui/widgets/app/app.viewmodel.js index ea9b0640b3..99a42319a2 100644 --- a/cui/widgets/app/app.viewmodel.js +++ b/cui/widgets/app/app.viewmodel.js @@ -84,8 +84,8 @@ define(['knockout','pubsub', 'utils'], function(ko,PubSub, utils) { } }; function getParameterByName(win, name) { - // codeql[js/incomplete-sanitization] justification: added /g flag to make escaping global; re-review by 2027-07-31 - name = name.replace(/[\[]/g, "\\[").replace(/[\]]/g, "\\]"); + // codeql[js/incomplete-sanitization] justification: added /g flag + backslash escape; re-review by 2027-07-31 + name = name.replace(/[\[\]\\]/g, "\\$&"); var regex = new RegExp("[\\?&]" + name + "=([^&#]*)"), results = regex.exec(win.location.search); return results == null ? "" : decodeURIComponent(results[1].replace(/\+/g, " ")); diff --git a/delivery/common/js/views/PercArchiveListView.js b/delivery/common/js/views/PercArchiveListView.js index 779dc3821c..a23a58d21f 100644 --- a/delivery/common/js/views/PercArchiveListView.js +++ b/delivery/common/js/views/PercArchiveListView.js @@ -134,6 +134,7 @@ encodedQuery = "&query=" + encodeURIComponent(JSON.stringify(query)); href = baseURL + pageResult + "?filter="+ encodeURIComponent(row.year) + encodedQuery; anchorYear = $("") + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 .attr("href",percSafeUrl(href)) .text(linkYearText); @@ -194,6 +195,7 @@ encodedQuery = "&query=" + encodeURIComponent(JSON.stringify(query)); href = baseURL + pageResult + "?filter="+ encodeURIComponent(row2.month + " " + row.year )+ encodedQuery; a = $("") + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 .attr("href",percSafeUrl(href) ) .text(linkText); } @@ -282,6 +284,7 @@ var encodedQuery = "&query=" + encodeURIComponent(JSON.stringify(query)); var href = baseURL + pageResult + "?filter="+ encodeURIComponent(row2.month +" "+ row.year) + encodedQuery; a = $("") + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 .attr("href", percSafeUrl(href)) .text(linkText); } diff --git a/delivery/common/js/views/PercBlogPostView.js b/delivery/common/js/views/PercBlogPostView.js index ab5c8b28d7..b08ac27bbc 100644 --- a/delivery/common/js/views/PercBlogPostView.js +++ b/delivery/common/js/views/PercBlogPostView.js @@ -34,7 +34,14 @@ updateBlogLink : updateBlogLink, trackBlogPost : trackBlogPost }; - + + function percSafeUrl(url) + { + var u = String(url); + if (/^\s*(?:javascript|vbscript|data)\s*:/i.test(u)) {return "#";} + return u; + } + function updateBlogNav() { $(".perc-blog-navigation-container").each(function(){ @@ -141,7 +148,8 @@ var tag = ($(this).text().trim()).replace(",", ""); var jsonQuery = {'criteria':["perc:tags LIKE '" + tag + "'"]}; var encodedQuery = "&query=" + encodeURIComponent(JSON.stringify(jsonQuery)); - $(this).attr("href", blogIndexPage + "?filter="+ encodeURIComponent(tag) + encodedQuery); + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 + $(this).attr("href", percSafeUrl(blogIndexPage + "?filter="+ encodeURIComponent(tag) + encodedQuery)); }); // Categories @@ -150,7 +158,8 @@ var category = ($(this).text().trim()).replace(",", ""); var jsonQuery = {'criteria':["perc:category LIKE '" + categoryPath + "'"]}; var encodedQuery = "&query=" + encodeURIComponent(JSON.stringify(jsonQuery)); - $(this).attr("href", blogIndexPage + "?filter="+ encodeURIComponent(category) + encodedQuery); + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 + $(this).attr("href", percSafeUrl(blogIndexPage + "?filter="+ encodeURIComponent(category) + encodedQuery)); }); }); } diff --git a/delivery/common/js/views/PercCategoryListView.js b/delivery/common/js/views/PercCategoryListView.js index aab3cf97cf..eadeae77fd 100644 --- a/delivery/common/js/views/PercCategoryListView.js +++ b/delivery/common/js/views/PercCategoryListView.js @@ -195,6 +195,7 @@ var isPreviewMode; } var a = $("") + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 .attr("href", percSafeUrl(href)) .attr("data-count", countTotal) .attr("title", nodeStr) diff --git a/delivery/common/js/views/PercRegistrationView.js b/delivery/common/js/views/PercRegistrationView.js index 6a5a71bec6..66f55f0325 100644 --- a/delivery/common/js/views/PercRegistrationView.js +++ b/delivery/common/js/views/PercRegistrationView.js @@ -79,6 +79,7 @@ if(!redirectUrl || "" === redirectUrl) { redirectUrl = "/"; } + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this location.href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 window.location.href=percSafeUrl(redirectUrl); } else { @@ -265,8 +266,9 @@ else { var params = ''; if ($.param.querystring()) { - params = '?' + $.param.querystring(); + params = '?' + $.param.querystring(); } + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this location sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 window.location = percSafeUrl(confirmation_page + params); } } diff --git a/delivery/common/js/views/PercTagListView.js b/delivery/common/js/views/PercTagListView.js index 82754c7cdf..26afba51a6 100644 --- a/delivery/common/js/views/PercTagListView.js +++ b/delivery/common/js/views/PercTagListView.js @@ -24,6 +24,13 @@ updateTagList : updateTagList }; + function percSafeUrl(url) + { + var u = String(url); + if (/^\s*(?:javascript|vbscript|data)\s*:/i.test(u)) {return "#";} + return u; + } + function updateTagList() { $(".perc-tag-list").each(function(){ @@ -112,7 +119,8 @@ var query = JSON.parse( strJSON ); query.criteria.push("perc:tags = '" + tagEntry.tagName + "'"); var encodedQuery = "&query=" + encodeURIComponent(JSON.stringify(query)); - newListElem.find("a").attr("href", baseURL + pageResult + "?filter="+encodeURIComponent(tagEntry.tagName) + encodedQuery).text(linkText); + // codeql[js/xss-through-dom] justification: percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 + newListElem.find("a").attr("href", percSafeUrl(baseURL + pageResult + "?filter="+encodeURIComponent(tagEntry.tagName) + encodedQuery)).text(linkText); } else{ newListElem.find("a").text(linkText); diff --git a/docs/ai-generated/tasks/8.1.x-codeql-baseline/suppressions.md b/docs/ai-generated/tasks/8.1.x-codeql-baseline/suppressions.md index 4329465b4b..f53fbed2b5 100644 --- a/docs/ai-generated/tasks/8.1.x-codeql-baseline/suppressions.md +++ b/docs/ai-generated/tasks/8.1.x-codeql-baseline/suppressions.md @@ -131,8 +131,8 @@ Row schema (per spec 004 C2 contract): | 563 | java/xss | .github/codeql/codeql-config.yml | 39 | vendored Tomcat sample app (Hello.java + JSP demos); runtime defense not applicable | 2026-08-15 | 2027-07-31 | this PR | path-level residual; tomcat-docs is a documentation mirror of the upstream Apache Tomcat distribution | | 564 | java/xss | .github/codeql/codeql-config.yml | 39 | vendored Tomcat sample app (Hello.java + JSP demos); runtime defense not applicable | 2026-08-15 | 2027-07-31 | this PR | path-level residual; same Hello.java class as #563 | | 334 | js/clear-text-logging | .github/codeql/codeql-config.yml | 25 | Playwright QA tests log env-derived values for debug; production runtime not affected | 2026-08-15 | 2027-07-31 | this PR | path-level residual; tests/login.spec.js:33 logs error.message and process.env-derived CMS credentials for debug; not in production | -| 234 | js/incomplete-sanitization | cui/widgets/app/app.viewmodel.js | 87 | added /g flag to make escaping global; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: replaced /[\[]/ with /[\[]/g to make escaping global; closes both #234 and #235 | -| 235 | js/incomplete-sanitization | cui/widgets/app/app.viewmodel.js | 87 | added /g flag to make escaping global; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: same line as #234; /g flag closes both alerts | +| 234 | js/incomplete-sanitization | cui/widgets/app/app.viewmodel.js | 87 | added /g flag + backslash escape; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: replaced /[\[]/ with /[\[]/g to make escaping global; closes both #234 and #235 | +| 235 | js/incomplete-sanitization | cui/widgets/app/app.viewmodel.js | 87 | added /g flag + backslash escape; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: same line as #234; /g flag closes both alerts | | 454 | java/path-injection | projects/sitemanage/src/main/java/com/percussion/sitemanage/importer/helpers/impl/PSImportThemeHelper.java | 257 | upstream PSURLConverter.getConvertedFileSystemPath canonicalizes via getCanonicalPath; GHAS does not model canonical-path-only defense; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line residual: removeIfExists checks file existence from imported-site path | | 455 | java/path-injection | projects/sitemanage/src/main/java/com/percussion/sitemanage/importer/theme/PSCSSParser.java | 364 | upstream PSURLConverter.getConvertedFileSystemPath canonicalizes via getCanonicalPath; GHAS does not model canonical-path-only defense; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line residual: fileExists opens file derived from imported-site URL path | | 456 | java/path-injection | projects/sitemanage/src/main/java/com/percussion/sitemanage/importer/theme/PSCSSParser.java | 420 | upstream PSURLConverter.getConvertedFileSystemPath canonicalizes via getCanonicalPath; GHAS does not model canonical-path-only defense; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line residual: saveFile opens FileWriter on imported-site path | @@ -142,3 +142,14 @@ Row schema (per spec 004 C2 contract): | 476 | java/path-injection | projects/sitemanage/src/main/java/com/percussion/theme/service/impl/PSThemeService.java | 521 | PSPathInjectionGuard.requireSafeFileName above; GHAS does not model in-repo sanitizer; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line residual: clearCacheRegionCSS uses safeSessionSegment-guarded session id | | 477 | java/path-injection | projects/sitemanage/src/main/java/com/percussion/theme/service/impl/PSThemeService.java | 522 | PSPathInjectionGuard.requireSafeFileName above; GHAS does not model in-repo sanitizer; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line residual: same method as #476; second FileUtils.deleteQuietly sink | | 704 | java/path-injection | projects/sitemanage/src/main/java/com/percussion/theme/service/impl/PSThemeService.java | 216 | PSPathInjectionGuard.requireSafeFileName above; GHAS does not model in-repo sanitizer; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line residual: getNewThemeFolder loop uses requireSafeFileName-guarded themeName | +| 705 | js/incomplete-sanitization | cui/widgets/app/app.viewmodel.js | 88 | added /g flag + backslash escape; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: regex now matches [\]\\] globally so escaping covers all bracket and backslash inputs; closes both #705 and #706 | +| 706 | js/incomplete-sanitization | cui/widgets/app/app.viewmodel.js | 88 | added /g flag + backslash escape; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: same line as #705; /g + backslash coverage closes both alerts | +| 707 | js/xss-through-dom | delivery/common/js/views/PercBlogPostView.js | 144 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: added percSafeUrl helper + wrapped .attr(href) call; encodeURIComponent already on tag value | +| 708 | js/xss-through-dom | delivery/common/js/views/PercBlogPostView.js | 153 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: same helper as #707; wrapped category href | +| 709 | js/xss-through-dom | delivery/common/js/views/PercArchiveListView.js | 137 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line suppression: PR #60 added percSafeUrl() wrapper but GHAS does not model it | +| 710 | js/xss-through-dom | delivery/common/js/views/PercArchiveListView.js | 197 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line suppression: same helper as #709 | +| 711 | js/xss-through-dom | delivery/common/js/views/PercArchiveListView.js | 285 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line suppression: same helper as #709 | +| 712 | js/xss-through-dom | delivery/common/js/views/PercCategoryListView.js | 198 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line suppression: same helper as #709 | +| 713 | js/xss-through-dom | delivery/common/js/views/PercRegistrationView.js | 82 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this location.href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line suppression: same helper as #709 | +| 714 | js/xss-through-dom | delivery/common/js/views/PercRegistrationView.js | 270 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this location sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | sink-line suppression: same helper as #709 | +| 715 | js/xss-through-dom | delivery/common/js/views/PercTagListView.js | 115 | percSafeUrl() helper blocks javascript:/vbscript:/data: schemes at this href sink; GHAS does not model the in-repo helper as a sanitizer barrier; re-review by 2027-07-31 | 2026-08-15 | 2027-07-31 | this PR | code fix: added percSafeUrl helper + wrapped .attr(href) call; encodeURIComponent already on tag value |