Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-06 23:27:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1181 from apache/fix/WW-5501-exclude

Browse Source 
WW-5501 Only exclude malicious file names
This commit is contained in:
Lukasz Lenart
2025-01-16 07:28:46 +01:00
committed by GitHub
parent c7a6daf5ce 688162cae2
commit cba0db0b07
5 changed files with 26 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+21 -13
View File
@@ -20,7 +20,8 @@ package org.apache.struts2.dispatcher.multipart;
import com.opensymphony.xwork2.LocaleProviderFactory;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker;
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
@@ -39,13 +40,15 @@ 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.
*/
public static final int BUFFER_SIZE = 10240;
/**
* Internal list of raised errors to be passed to the the Struts2 framework.
* Internal list of raised errors to be passed to the Struts2 framework.
*/
protected List<LocalizedMessage> errors = new ArrayList<>();
@@ -80,7 +83,13 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
* Localization to be used regarding errors.
*/
protected Locale defaultLocale = Locale.ENGLISH;
private 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.
@@ -123,14 +132,9 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
defaultLocale = localeProviderFactory.createLocaleProvider().getLocale();
}
@Inject
public void setNotExcludedAllowedPatternsChecker(NotExcludedAcceptedPatternsChecker patternsChecker) {
this.patternsChecker = patternsChecker;
}
/**
* @param request Inspect the servlet request and set the locale if one wasn't provided by
* the Struts2 framework.
* the Struts2 framework.
*/
protected void setLocale(HttpServletRequest request) {
if (defaultLocale == null) {
@@ -141,7 +145,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
/**
* Build error message.
*
* @param e the Throwable/Exception
* @param e the Throwable/Exception
* @param args arguments
* @return error message
*/
@@ -154,7 +158,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
/* (non-Javadoc)
* @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors()
*/
*/
public List<LocalizedMessage> getErrors() {
return errors;
}
@@ -176,8 +180,12 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
return fileName;
}
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();
}
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+3 -3
View File
@@ -115,12 +115,12 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
protected void processFileField(FileItem item) {
LOG.debug("Item is a file upload");
if (!isAccepted(item.getName())) {
if (isExcluded(item.getName())) {
LOG.warn("File name [{}] is not accepted", normalizeSpace(item.getName()));
return;
}
if (!isAccepted(item.getFieldName())) {
if (isExcluded(item.getFieldName())) {
LOG.warn("Field name [{}] is not accepted", normalizeSpace(item.getFieldName()));
return;
}
@@ -146,7 +146,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
try {
LOG.debug("Item is a normal form field");
if (!isAccepted(item.getFieldName())) {
if (isExcluded(item.getFieldName())) {
LOG.warn("Form field name [{}] is not accepted", normalizeSpace(item.getFieldName()));
return;
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
+2 -3
View File
@@ -25,7 +25,6 @@ import org.apache.commons.fileupload.FileUploadBase.FileSizeLimitExceededExcepti
import org.apache.commons.fileupload.FileUploadException;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.fileupload.util.Streams;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.dispatcher.LocalizedMessage;
@@ -315,7 +314,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
*/
protected void processFileItemStreamAsFormField(FileItemStream itemStream) {
String fieldName = itemStream.getFieldName();
if (!isAccepted(fieldName)) {
if (isExcluded(fieldName)) {
LOG.warn("Form field [{}] rejected!", normalizeSpace(fieldName));
return;
}
@@ -347,7 +346,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
return;
}
if (!isAccepted(itemStream.getName())) {
if (isExcluded(itemStream.getName())) {
LOG.warn("File field [{}] rejected", normalizeSpace(itemStream.getName()));
return;
}
core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
-4
View File
@@ -767,15 +767,11 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
jak.setMaxFileSize(String.valueOf(maxfilesize));
jak.setMaxFiles(String.valueOf(maxfiles));
jak.setMaxStringLength(String.valueOf(maxStringLength));
DefaultNotExcludedAcceptedPatternsChecker patternsChecker = container.inject(DefaultNotExcludedAcceptedPatternsChecker.class);
jak.setNotExcludedAllowedPatternsChecker(patternsChecker);
return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}
private MultiPartRequestWrapper createMultipartRequestNoMaxParamsSet(HttpServletRequest req) {
JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
DefaultNotExcludedAcceptedPatternsChecker patternsChecker = container.inject(DefaultNotExcludedAcceptedPatternsChecker.class);
jak.setNotExcludedAllowedPatternsChecker(patternsChecker);
return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}
core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
-4
View File
@@ -829,16 +829,12 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
jak.setMaxFileSize(String.valueOf(maxfilesize));
jak.setMaxFiles(String.valueOf(maxfiles));
jak.setMaxStringLength(String.valueOf(maxStringLength));
DefaultNotExcludedAcceptedPatternsChecker patternsChecker = container.inject(DefaultNotExcludedAcceptedPatternsChecker.class);
jak.setNotExcludedAllowedPatternsChecker(patternsChecker);
return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}
private MultiPartRequestWrapper createMultipartRequestNoMaxParamsSet(HttpServletRequest req) {
JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
DefaultNotExcludedAcceptedPatternsChecker patternsChecker = container.inject(DefaultNotExcludedAcceptedPatternsChecker.class);
jak.setNotExcludedAllowedPatternsChecker(patternsChecker);
return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}