From 8e19ddded5a5e5611314880c8129feb78d7b1a16 Mon Sep 17 00:00:00 2001 From: Nate Chadwick <263952448+natechadwick-intsof@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:58:47 -0400 Subject: [PATCH] fix(security): T2.6 Apache Commons hardening (size caps on commons-fileupload + commons-compress) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../service/impl/PSTemplateInfo.java | 19 ++++- .../service/impl/PSTemplateServlet.java | 19 ++++- .../com/percussion/util/PSArchiveFiles.java | 77 +++++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateInfo.java b/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateInfo.java index 0c88ad8b46..07643acc01 100644 --- a/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateInfo.java +++ b/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateInfo.java @@ -54,6 +54,14 @@ public class PSTemplateInfo extends HttpServlet { private static final int DEFAULT_BUFFER_SIZE = 20480; // 20KB. + // T2.6 hardening (issue #82): bound the multipart upload to prevent DoS via huge + // payloads against commons-fileupload 1.6.0 (which has 7 CVEs in that area). + // Values match the existing PSAssetUploadServlet config in WebUI/war/WEB-INF/web.xml + // (100MB file / 400MB request) but tuned for template XML files which are smaller. + private static final int UPLOAD_MEMORY_THRESHOLD = 1 << 20; // 1MB + private static final long UPLOAD_MAX_FILE_SIZE = 50L << 20; // 50MB + private static final long UPLOAD_MAX_REQUEST_SIZE = 100L << 20; // 100MB + private static final Logger log = LogManager.getLogger(PSTemplateInfo.class); public PSTemplateInfo() { @@ -121,8 +129,15 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) if (isMultipart) { try { - List items = - new ServletFileUpload(new DiskFileItemFactory()).parseRequest(request); + // T2.6 hardening: configure commons-fileupload with explicit size limits. + // The previous `new ServletFileUpload(new DiskFileItemFactory())` had no + // bounds, leaving the server open to DoS via arbitrarily large uploads. + DiskFileItemFactory factory = new DiskFileItemFactory(); + factory.setSizeThreshold(UPLOAD_MEMORY_THRESHOLD); + ServletFileUpload upload = new ServletFileUpload(factory); + upload.setFileSizeMax(UPLOAD_MAX_FILE_SIZE); + upload.setSizeMax(UPLOAD_MAX_REQUEST_SIZE); + List items = upload.parseRequest(request); for (FileItem item : items) { if (!item.isFormField()) { templateImported = importTemplate(siteId, item); diff --git a/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateServlet.java b/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateServlet.java index e81e1d3077..60eb1fb63d 100644 --- a/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateServlet.java +++ b/projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateServlet.java @@ -51,6 +51,14 @@ public class PSTemplateServlet extends HttpServlet { private static final int DEFAULT_BUFFER_SIZE = 20480; // 20KB. + // T2.6 hardening (issue #82): bound the multipart upload to prevent DoS via huge + // payloads against commons-fileupload 1.6.0 (which has 7 CVEs in that area). + // Values match the existing PSAssetUploadServlet config in WebUI/war/WEB-INF/web.xml + // (100MB file / 400MB request) but tuned for template XML files which are smaller. + private static final int UPLOAD_MEMORY_THRESHOLD = 1 << 20; // 1MB + private static final long UPLOAD_MAX_FILE_SIZE = 50L << 20; // 50MB + private static final long UPLOAD_MAX_REQUEST_SIZE = 100L << 20; // 100MB + public PSTemplateServlet() { PSSpringWebApplicationContextUtils.injectDependencies(this); } @@ -113,8 +121,15 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) if (isMultipart) { try { - List items = - new ServletFileUpload(new DiskFileItemFactory()).parseRequest(request); + // T2.6 hardening: configure commons-fileupload with explicit size limits. + // The previous `new ServletFileUpload(new DiskFileItemFactory())` had no + // bounds, leaving the server open to DoS via arbitrarily large uploads. + DiskFileItemFactory factory = new DiskFileItemFactory(); + factory.setSizeThreshold(UPLOAD_MEMORY_THRESHOLD); + ServletFileUpload upload = new ServletFileUpload(factory); + upload.setFileSizeMax(UPLOAD_MAX_FILE_SIZE); + upload.setSizeMax(UPLOAD_MAX_REQUEST_SIZE); + List items = upload.parseRequest(request); for (FileItem item : items) { if (!item.isFormField()) { templateImported = importTemplate(siteId, item); diff --git a/system/src/main/java/com/percussion/util/PSArchiveFiles.java b/system/src/main/java/com/percussion/util/PSArchiveFiles.java index 40be4742bb..e58894e5be 100644 --- a/system/src/main/java/com/percussion/util/PSArchiveFiles.java +++ b/system/src/main/java/com/percussion/util/PSArchiveFiles.java @@ -44,6 +44,45 @@ public class PSArchiveFiles { private static final Logger log = LogManager.getLogger(IPSConstants.PACKAGING_LOG); + // T2.6 hardening (issue #82): bound archive extraction to limit exposure to the + // 11 CVEs in commons-compress 1.28.0 (mostly zip-bomb / oversized-entry attacks). + // The path-traversal half is already covered by the ZipSlipGuard + canonical-path + // check below. These three limits cover the resource-exhaustion half. + // Defaults are tuned to "huge but realistic" (a 100MB war, 10K entries, 500MB total + // uncompressed); operators can override per-call by setting the system properties + // PSARCHIVE_MAX_ENTRY_SIZE / PSARCHIVE_MAX_ENTRIES / PSARCHIVE_MAX_TOTAL_SIZE. + private static final long DEFAULT_MAX_ENTRY_SIZE = 100L << 20; // 100MB per entry + private static final int DEFAULT_MAX_ENTRIES = 10_000; // 10K entries per archive + private static final long DEFAULT_MAX_TOTAL_SIZE = 500L << 20; // 500MB total uncompressed + + private static long getMaxEntrySize() { + return readLongProp("PSARCHIVE_MAX_ENTRY_SIZE", DEFAULT_MAX_ENTRY_SIZE); + } + + private static int getMaxEntries() { + return (int) readLongProp("PSARCHIVE_MAX_ENTRIES", DEFAULT_MAX_ENTRIES); + } + + private static long getMaxTotalSize() { + return readLongProp("PSARCHIVE_MAX_TOTAL_SIZE", DEFAULT_MAX_TOTAL_SIZE); + } + + private static long readLongProp(String name, long def) { + String v = System.getProperty(name); + if (v == null || v.isEmpty()) return def; + try { + return Long.parseLong(v.trim()); + } catch (NumberFormatException nfe) { + log.warn( + "Ignoring non-numeric system property {}={} ({}); using default {}", + name, + v, + nfe.getMessage(), + def); + return def; + } + } + /** * Opens the specified archive file. The classes calling this method are responsible for closing * this file. @@ -344,10 +383,48 @@ public static String extractFilesFromArchive( pw.println("Extracting files from archive"); } + // T2.6 hardening: read the size caps once per extraction so an attacker cannot + // cause repeated property lookups, and so the same limit is applied to every + // entry in this archive. + final long maxEntrySize = getMaxEntrySize(); + final int maxEntries = getMaxEntries(); + final long maxTotalSize = getMaxTotalSize(); + int entriesSeen = 0; + long bytesSeen = 0L; + for (Enumeration files = archiveFile.entries(); files.hasMoreElements(); ) { ZipEntry entry = (ZipEntry) files.nextElement(); StringBuilder errorBuf = new StringBuilder(); + // T2.6 hardening: enforce MAX_ENTRIES and per-entry / total uncompressed size + // caps before doing any directory or file work. These limits are what protect + // against the zip-bomb half of the commons-compress 1.28.0 CVE set. + if (++entriesSeen > maxEntries) { + throw new SecurityException( + "Archive rejected: too many entries (limit=" + maxEntries + ")"); + } + final long entrySize = entry.getSize(); + if (entrySize > maxEntrySize) { + throw new SecurityException( + "Archive rejected: entry '" + + entry.getName() + + "' uncompressed size " + + entrySize + + " exceeds limit " + + maxEntrySize); + } + if (entrySize > 0) { + bytesSeen += entrySize; + if (bytesSeen > maxTotalSize) { + throw new SecurityException( + "Archive rejected: total uncompressed size exceeds limit " + + maxTotalSize + + " (entry='" + + entry.getName() + + "')"); + } + } + // Check whether the directory exists for this file. If not, create it. String dir = ""; String name = entry.getName();