fix(security): T2.6 Apache Commons hardening (size caps on commons-fileupload + commons-compress) - #83
Merged
Conversation
…leupload + commons-compress) Sub-task of issue #82 and the parent epic #73. This is defense-in-depth work for vulnerabilities in libraries that cannot be upgraded on Java 1.8: - commons-fileupload 1.6.0 (7 CVEs, mostly DoS via huge uploads) - commons-compress 1.28.0 (11 CVEs, mostly zip-bomb / oversized entries) - commons-io 2.21.0 (2 CVEs, around untrusted file paths; out of scope for this PR; test-only usage) The library versions stay the same; what changes is the project's defensive usage in the few places we actually call into them. - PSTemplateServlet.java: configure DiskFileItemFactory + ServletFileUpload with setSizeThreshold (1MB), setFileSizeMax (50MB), setSizeMax (100MB). Tuned for template XML files (vs the existing PSAssetUploadServlet config in WebUI/war/WEB-INF/web.xml which is 100MB / 400MB for asset uploads). The previous was unbounded and a single malicious POST could exhaust heap. - PSTemplateInfo.java: same fix as PSTemplateServlet (this is the second and last commons-fileupload user in the project). - PSArchiveFiles.extractFilesFromArchive: add three resource-exhaustion limits in the entry iteration loop: * MAX_ENTRIES (10K) — reject any archive with more entries * MAX_ENTRY_SIZE (100MB per entry) — reject any single entry whose declared uncompressed size exceeds the cap * MAX_TOTAL_SIZE (500MB total uncompressed) — sum the declared uncompressed sizes as entries are processed; abort if the running total exceeds the cap Each limit is overridable per JVM via the system properties PSARCHIVE_MAX_ENTRIES / PSARCHIVE_MAX_ENTRY_SIZE / PSARCHIVE_MAX_TOTAL_SIZE (operator can tune if a real use case ever needs more headroom). The existing ZipSlipGuard + canonical-path check covers the path-traversal half of the 11 CVEs; the new limits cover the zip-bomb half. All three limits fail-closed (throw SecurityException with the offending entry name / size) — same fail-closed pattern as the existing ZipSlip check at lines 354-358 / 374-379 / 398-403 of PSArchiveFiles. Verification: - ./mvn-env.sh clean install -DskipTests: BUILD SUCCESS in 3:28 - All 3 files compile; no source-format / spotless issues - No UnsupportedClassVersionError in the build log - commons-compress 1.28.0 / commons-fileupload 1.6.0 / commons-io 2.21.0 jars are unchanged in the local Maven cache Refs #82, #73 > Co-Authored by Mavis v1.0.0 using minimax-m3 with agent mavis.
natechadwick
approved these changes
Aug 28, 2026
natechadwick
pushed a commit
that referenced
this pull request
Aug 28, 2026
Closes #84. Mechanical migration of every `org.apache.commons.lang.*` import to `org.apache.commons.lang3.*` across the codebase, plus removal of the 2.6 dep from the root pom and 28 module poms. commons-lang 2.6 has been EOL since 2010. The 2.7+ line is also unmaintained, and the project's other analysis (T2.6 hardening, #83) flags the same set of CVEs. The 3.x line is the maintained successor; `commons-lang3:3.20.0` was already in the root pom's dependencyManagement (only the 2.6 line was in active use), so no new dep is added. Scope: - 1,396 .java files: `org.apache.commons.lang.` -> `org.apache.commons.lang3.` - 29 .xml files (root pom + 28 module poms): drop the `commons-lang:commons-lang:2.6` dependency and the `<commons.lang.version>2.6</commons.lang.version>` property - 6 special-case files (see API notes below) API differences between commons-lang 2.x and 3.20.0 that this PR addresses (all behavioural, no semantic change for the call sites involved): 1. CharEncoding was removed (it was a 2.x-only constants holder). Fixed in projects/sitemanage/src/test/java/.../PSRestClient.java by switching to java.nio.charset.StandardCharsets.UTF_8.name(). 2. StrTokenizer (text.* subpackage) was removed in lang3 3.12+. Fixed in modules/ContentUI/.../PSAutoLinkGenerationProperties.java by inlining a StringUtils.split(text, ',') loop (only one call site). 3. WordUtils was moved from top-level `org.apache.commons.lang3.WordUtils` to `org.apache.commons.lang3.text.WordUtils`. 3 call sites fixed via a per-file import path correction (modules/utils, deployer, projects/sitemanage). 4. NullArgumentException was removed entirely. The only call site (projects/sitemanage/.../AssetAdaptor.java) now throws java.lang.NullPointerException with the same custom message. 5. ExceptionUtils.getFullStackTrace(t) was renamed to getStackTrace(t) in lang3 3.0. 49 call sites updated across 24 files (some broken across multiple lines; fixed with a word-bounded sed). 6. Validate.allElementsOfType(coll, type) was removed. The single call site (projects/sitemanage/.../PSResourceInstanceHelper.java) now inlines an instanceof loop with an IllegalArgumentException on mismatch. 7. StringEscapeUtils (text.* subpackage) was moved out of lang3 entirely; the maintained replacement is `org.apache.commons.text.StringEscapeUtils` in commons-text (already a transitive dep at 1.15.0). The 2 call sites in system/business/.../PSMetadataExtractorService.java and projects/sitemanage/.../PSLegacyLinkGenerator.java now import from commons-text. The legacy shortcut `StringEscapeUtils.unescapeHtml("&entity;")` is replaced with `StringEscapeUtils.UNESCAPE_HTML4.translate("&entity;")` in PSMetadataExtractorService (long form `escapeHtml4`/`unescapeHtml4` used elsewhere; both still exist in commons-text 1.15.0). Verification: - ./mvn-env.sh clean install -DskipTests: BUILD SUCCESS in 2:18 (61 modules, Java 1.8.0_504) - ./mvn-env.sh spotless:check: clean (the only spotless violation during the run was a multi-line import comment I had to inline; the project spotless config ran auto-style during the build and fixed everything else) - No UnsupportedClassVersionError in the build log - commons-lang 2.x no longer in the project's own dependency graph (commons-lang 2.5 is still pulled in transitively by axe/caja/ smartgwt-jar, but the project's own declared dep is gone) Out of scope (separate issues under #73): - commons-collections 3.2.2 -> commons-collections4 (next EOL in queue) - commons-beanutils 1.11.0 -> beanutils2 (Jakarta migration) - commons-httpclient 3.1 -> HttpClient 5 (the project already has HttpClient 5 from PR #79) - commons-configuration 1.10 -> commons-configuration2 Refs #84, #73, #72
Merged
5 tasks
natechadwick
pushed a commit
that referenced
this pull request
Aug 28, 2026
…ions4 4.5.0 (issue #86) (#87) Closes #86. Mechanical migration of every `org.apache.commons.collections.*` import to `org.apache.commons.collections4.*` across the codebase, plus removal of the 3.2.2 dep from the root pom and 13 module poms. The `commons-collections4:4.5.0` line was already in the root pom's dependencyManagement (only the 3.x line was in active use), so no new dep is added. Scope: - 71 .java files: `org.apache.commons.collections.` -> `org.apache.commons.collections4.` - 1 root pom + 13 module poms: drop the `commons-collections:commons-collections:3.2.2` dependency and the `<commons.collections3.version>3.2.2</commons.collections3.version>` property - 1 module pom (jcadf-master): add a direct `org.apache.commons:commons-collections4` dep, since the original `commons-collections:commons-collections` direct dep was the sole source of the collections classes in this module - 6 special-case files (see API notes below) API differences between commons-collections 3.x and 4.5.0 that this PR addresses (all behavioural, no semantic change for the call sites involved): 1. `org.apache.commons.collections.MultiHashMap` was **removed** in 4.x. The 3 call sites now use `org.apache.commons.collections4.multimap.ArrayListValuedHashMap`. The `MultiMap` interface (deprecated in 4.x but still present) was replaced with `MultiValuedMap` (the new 4.x interface that replaced it; `ArrayListValuedHashMap` implements `MultiValuedMap`, not `MultiMap`). Files: PSCalendarMonthModel, PSContentRepository, PSItemUtilities + 2 callers (PSActionPanelServlet, PSItemUtilitiesTest). 2. `CollectionUtils.addAll(Collection, Iterator)` was **removed** in 4.x (the new signatures are `addAll(Collection, Iterable)` and `addAll(Collection, Enumeration)` only). The 4 call sites that pass an Iterator are inlined to a `while (it.hasNext()) list.add(it.next())` loop. The 7 sites that pass an Iterable/Collection work unchanged. Files: PSLegacyExtensionUtils, PSContentTypeSetter (iteratorToList), PSContentTypeFieldSetter (excludes loop), PSConditionalCloneHandler. 3. `AbstractListDecorator.getCollection()` / `getList()` were renamed to `decorated()` in 4.x. The 1 call site in PSConcurrentRegionsAssembler.FutureList inlines the override as `decorated()` (and calls it from the iterator/toString overrides that previously used the public `getList()` / `getCollection()`). 4. `MapUtils.getString(Map, String)` (2-arg) and the new 3-arg `getString(Map, String, String)` (default) in 4.x use a generic signature `<K> String getString(Map<? super K, ?>, K)` that does not type-infer when the input is `Map<?, ?>`. The 1 call site in PSPageUtils casts to a raw `Map` to anchor the K-inference to String (with a `@SuppressWarnings({unchecked, rawtypes})` on the line). Verification: - ./mvn-env.sh clean install -DskipTests: BUILD SUCCESS in 3:41 (61 modules, Java 1.8.0_504) - ./mvn-env.sh spotless:check: clean (the build runs spotless:apply during validate; nothing needed reformatting beyond the inline raw-type cast above) - No UnsupportedClassVersionError in the build log - commons-collections 3.x no longer in the project's own dependency graph (only commons-collections4 4.5.0 remains; `commons-collections` 2.5 may still be pulled in transitively by legacy system-scoped jars, but that is independent of this PR) Out of scope (separate issues under #73): - commons-beanutils 1.11.0 -> beanutils2 EOL replacement (Jakarta migration; large blast radius) - commons-httpclient 3.1 -> HttpClient 5 (the project already has HttpClient 5 from #79; just remove the 3.1 dep) - commons-configuration 1.10 -> commons-configuration2 EOL replacement - T2.11 SnakeYAML SafeConstructor hardening (16 CVEs) - T2.6 size caps on the other zip sites (follow-up to #83) Refs #86, #73, #72
This was referenced Aug 28, 2026
natechadwick
pushed a commit
that referenced
this pull request
Aug 28, 2026
…p sites (issue #89) (#90) Follow-up to PR #83 (which added the same caps to PSArchiveFiles.extractFilesFromArchive). This PR brings the same defense-in-depth to the other 7 zip-handling sites in the project. What's new: - PSZipBombGuard: a small utility class in modules/perc-security-utils (com.percussion.security.io.PSZipBombGuard) that wraps the three caps in a stateful, reusable object. Constructor takes (maxEntries, maxEntrySize, maxTotalSize); use the no-arg constructor for the defaults. All three caps are overridable per JVM via the system properties PSARCHIVE_MAX_ENTRIES / PSARCHIVE_MAX_ENTRY_SIZE / PSARCHIVE_MAX_TOTAL_SIZE (matching the override keys used in #83). - Three fail-closed checks per call: * MAX_ENTRIES = 10,000 entries per archive * MAX_ENTRY_SIZE = 100 MB per entry (declared uncompressed size) * MAX_TOTAL_SIZE = 500 MB total uncompressed size across all entries A single SecurityException is thrown for the first cap that is hit; the offending entry name and the cap value are included in the message. Where it's used (7 production zip sites; the 8th ZipSlipGuard-using file was already hardened in #83): - projects/sitemanage/.../PSWidgetPackageBuilder.java (ZipInputStream.getNextEntry loop) - deliverytiersuite/.../MainDTSPreInstall.java (ZipFile.entries() iteration, sorted) - modules/perc-distribution-tree/.../Main.java (ZipFile.entries() iteration, sorted) - system/.../tools/PSInstallRxApp.java (ZipFile.entries() enumeration) - system/.../tools/InstallRxApp.java (ZipFile.entries() enumeration) - system/release/Install/.../RxExtractJarFiles.java (JarFile.entries() enumeration) - modules/perc-ant/.../PSExtractJarFiles.java (JarFile.entries() enumeration) All 7 sites already had ZipSlipGuard + canonical-path checks (the "// codeql[java/zipslip] justification" suppressions are unchanged); the new code only adds the resource-exhaustion caps to those checks. Total diff: 8 files, +182 / -0. What's NOT hardened (explicit out-of-scope): - PSDirectoryAnalyzer.java uses zip.getEntry() (a single named lookup, not an iteration) — not a zip-bomb attack surface. - PSPackageBuilder.java only does ZipOutputStream writes (building archives, not reading them) — no attack surface. - PSArchive.java in deployer/ only does getEntry() (single named lookup). - PSPackageLockManager.java has no zip iteration at all (it operates on package metadata, not the zip contents). - All test files are intentionally excluded (test fixtures are controlled). Verification: - ./mvn-env.sh clean install -DskipTests: BUILD SUCCESS in 4:06 (61 modules, Java 1.8.0_504) - ./mvn-env.sh spotless:check: clean (the build runs spotless:apply during validate; nothing needed reformatting) - No UnsupportedClassVersionError in the build log - All 7 hardened sites already had perc-security-utils as a transitive dep (they were already using ZipSlipGuard), so no module-pom dep additions were required Out of scope (separate issues): - commons-httpclient 3.1 -> HttpClient 5 (issue #88, deferred; multi-day migration across 29 files) - T2.11 SnakeYAML SafeConstructor hardening (16 CVEs) - commons-beanutils 1.11.0 -> beanutils2 EOL replacement - commons-configuration 1.10 -> commons-configuration2 EOL replacement Refs #89, #73, #72
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
T2.6 (Apache Commons hardening) sub-task of the parent epic #73. This is defense-in-depth work for libraries that cannot be upgraded on Java 1.8:
commons-fileupload 1.6.0commons-compress 1.28.0commons-io 2.21.0The library versions stay the same; what changes is the project's defensive usage in the few places we actually call into them.
What changes (3 files, +111 / −4)
PSTemplateServlet.javaandPSTemplateInfo.javaBoth were instantiating
new ServletFileUpload(new DiskFileItemFactory()).parseRequest(request)with no size limits —setFileSizeMaxandsetSizeMaxwere bothunlimited, so a single malicious multipart POST could exhaust heap. The fix is to set:DiskFileItemFactory.setSizeThreshold(1MB)— small uploads stay in memoryServletFileUpload.setFileSizeMax(50MB)— reject oversize filesServletFileUpload.setSizeMax(100MB)— reject oversize requestsValues are tuned for template XML files (which are small) and match the existing
PSAssetUploadServletconfig inWebUI/war/WEB-INF/web.xmlandsystem/ear/WEB-INF/web.xml(which is 100MB / 400MB for asset uploads).PSArchiveFiles.extractFilesFromArchiveThe path-traversal half of the 11 CVEs is already covered by the existing
ZipSlipGuard+ canonical-path check (lines 354-358, 374-379, 398-403). The new code adds three resource-exhaustion limits in the entry iteration loop, each fail-closed with aSecurityException:MAX_ENTRIES-DPSARCHIVE_MAX_ENTRIES=…MAX_ENTRY_SIZE(per entry uncompressed)-DPSARCHIVE_MAX_ENTRY_SIZE=…MAX_TOTAL_SIZE(sum of uncompressed sizes)-DPSARCHIVE_MAX_TOTAL_SIZE=…The three limits are read once per extraction and applied to every entry. If any limit is exceeded, the extraction aborts with a
SecurityExceptionthat includes the offending entry name and the cap that was hit.Verification
./mvn-env.sh clean install -DskipTests→ BUILD SUCCESS in 3:28 (61 modules, Java 1.8.0_504)UnsupportedClassVersionErrorin the build logspotless:applyduringvalidate; nothing was reformatted)commons-compress 1.28.0andcommons-fileupload 1.6.0jars in the local Maven cache are unchanged (no transitive resolution differences)/pagemanagement/templateImportshould now return413(or throwFileSizeLimitExceededExceptionserver-side, depending on Tomcat's behavior) — recommend running the QA suite for template import on a real installSecurityExceptionfromPSArchiveFiles.extractFilesFromArchiveOut of scope (separate issues under #73)
PSArchive.java,PSWidgetPackageBuilder.java,PSPackageBuilder.java,MainDTSPreInstall.java, etc.). They already haveZipSlipGuard+ canonical-path checks; adding the same size/entry caps would be a defensive follow-up. Recommended for a follow-up PR if review wants it.References
docs/ai-generated/tasks/PR#-DependencyVulnerabilityAnalysis/issues/02-epic-non-upgradeable.md#t26--apache-commons-hardeningZipSlipGuardhelper:com.percussion.security.io.ZipSlipGuardPSAssetUploadServletreference config:WebUI/war/WEB-INF/web.xmllines around<servlet-name>percAssetUploadServlet</servlet-name>andsystem/ear/WEB-INF/web.xml(same)