This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5501-exclude-s7
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 3d3b0761727d22ba9eef27a660703aa2ba1eeff6
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Fri Jan 10 07:45:57 2025 +0100
WW-5501 Only exclude malicious file names
---
.../multipart/AbstractMultiPartRequest.java | 26 ++++++++++++++--------
.../multipart/JakartaMultiPartRequest.java | 6 ++---
.../multipart/JakartaStreamMultiPartRequest.java | 4 ++--
.../ActionFileUploadInterceptorTest.java | 2 --
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
index aeace3120..b6b7f2682 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
@@ -19,6 +19,8 @@
package org.apache.struts2.dispatcher.multipart;
import org.apache.struts2.inject.Inject;
+import org.apache.struts2.security.DefaultExcludedPatternsChecker;
+import org.apache.struts2.security.ExcludedPatternsChecker;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException;
import org.apache.commons.fileupload2.core.FileUploadContentTypeException;
@@ -31,7 +33,6 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.dispatcher.LocalizedMessage;
-import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker;
import java.io.IOException;
import java.nio.charset.Charset;
@@ -53,6 +54,8 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
private static final Logger LOG =
LogManager.getLogger(AbstractMultiPartRequest.class);
+ private static final String EXCLUDED_FILE_PATTERN =
".*[<>&\"'|;\\\\/?*:]+.*|.*\\.\\..*";
+
/**
* Defines the internal buffer size used during streaming operations.
*/
@@ -108,7 +111,13 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
*/
protected Map<String, List<String>> parameters = new HashMap<>();
- protected NotExcludedAcceptedPatternsChecker patternsChecker;
+
+ private final ExcludedPatternsChecker patternsChecker;
+
+ public AbstractMultiPartRequest() {
+ patternsChecker = new DefaultExcludedPatternsChecker();
+ ((DefaultExcludedPatternsChecker)
patternsChecker).setAdditionalExcludePatterns(EXCLUDED_FILE_PATTERN);
+ }
/**
* @param bufferSize Sets the buffer size to be used.
@@ -183,11 +192,6 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
return Charset.forName(charsetStr);
}
- @Inject
- public void
setNotExcludedAllowedPatternsChecker(NotExcludedAcceptedPatternsChecker
patternsChecker) {
- this.patternsChecker = patternsChecker;
- }
-
/**
* Creates an instance of {@link JakartaServletDiskFileUpload} used by the
parser to extract uploaded files
*
@@ -425,8 +429,12 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
}
}
- protected boolean isAccepted(String fileName) {
- return patternsChecker.isAllowed(fileName).isAllowed();
+ /**
+ * @param fileName file name to check
+ * @return true if the file name is excluded
+ */
+ protected boolean isExcluded(String fileName) {
+ return patternsChecker.isExcluded(fileName).isExcluded();
}
}
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
index b36e6e2f8..737ba23f8 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
@@ -79,7 +79,7 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
protected void processNormalFormField(DiskFileItem item, Charset charset)
throws IOException {
LOG.debug("Item: {} is a normal form field", item.getName());
- if (!isAccepted(item.getFieldName())) {
+ if (isExcluded(item.getFieldName())) {
LOG.warn(() -> "Form field [%s] is
rejected!".formatted(normalizeSpace(item.getFieldName())));
return;
}
@@ -105,12 +105,12 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
}
protected void processFileField(DiskFileItem item) {
- if (!isAccepted(item.getName())) {
+ if (isExcluded(item.getName())) {
LOG.warn(() -> "File name [%s] is not
accepted".formatted(normalizeSpace(item.getName())));
return;
}
- if (!isAccepted(item.getFieldName())) {
+ if (isExcluded(item.getFieldName())) {
LOG.warn(() -> "Field name [%s] is not
accepted".formatted(normalizeSpace(item.getFieldName())));
return;
}
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
index 7fb44f21f..2d2b67c68 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
@@ -116,7 +116,7 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
String fieldName = fileItemInput.getFieldName();
String fieldValue = readStream(fileItemInput.getInputStream());
- if (!isAccepted(fieldName)) {
+ if (isExcluded(fieldName)) {
LOG.warn(() -> "Form field [%s] is
rejected!".formatted(normalizeSpace(fieldName)));
return;
}
@@ -198,7 +198,7 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
return;
}
- if (!isAccepted(fileItemInput.getName())) {
+ if (isExcluded(fileItemInput.getName())) {
LOG.warn(() -> "File field [%s]
rejected".formatted(normalizeSpace(fileItemInput.getName())));
return;
}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index 9aca62717..84f6c12db 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -610,8 +610,6 @@ public class ActionFileUploadInterceptorTest extends
StrutsInternalTestCase {
jak.setMaxFiles(String.valueOf(maxfiles));
jak.setMaxStringLength(String.valueOf(maxStringLength));
jak.setDefaultEncoding(StandardCharsets.UTF_8.name());
- DefaultNotExcludedAcceptedPatternsChecker patternsChecker =
container.inject(DefaultNotExcludedAcceptedPatternsChecker.class);
- jak.setNotExcludedAllowedPatternsChecker(patternsChecker);
return new MultiPartRequestWrapper(jak, request,
tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}
