[jira] [Work logged] (WW-5501) Exclude not malicious names

[ASF GitHub Bot (Jira)]

     [ 
https://issues.apache.org/jira/browse/WW-5501?focusedWorklogId=949739&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-949739
 ]

ASF GitHub Bot logged work on WW-5501:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 23/Dec/24 10:09
            Start Date: 23/Dec/24 10:09
    Worklog Time Spent: 10m 
      Work Description: github-advanced-security[bot] commented on code in PR 
#1156:
URL: https://github.com/apache/struts/pull/1156#discussion_r1895565528


##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java:
##########
@@ -113,6 +113,16 @@
     protected void processFileField(FileItem item) {
         LOG.debug("Item is a file upload");
 
+        if (!isAccepted(item.getName())) {
+            LOG.warn("File name [{}] is not accepted", item.getName());
+            return;
+        }
+
+        if (!isAccepted(item.getFieldName())) {
+            LOG.warn("Field name [{}] is not accepted", item.getFieldName());

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AZPy_bBpEosXzZdCgNBv-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AZPy_bBpEosXzZdCgNBv&open=AZPy_bBpEosXzZdCgNBv&pullRequest=1156";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/729)



##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java:
##########
@@ -312,6 +312,10 @@
      */
     protected void processFileItemStreamAsFormField(FileItemStream itemStream) 
{
         String fieldName = itemStream.getFieldName();
+        if (!isAccepted(fieldName)) {
+            LOG.warn("Form field [{}] rejected!", fieldName);

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AZPy_bDOEosXzZdCgNCE-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AZPy_bDOEosXzZdCgNCE&open=AZPy_bDOEosXzZdCgNCE&pullRequest=1156";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/731)



##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java:
##########
@@ -340,6 +344,11 @@
             return;
         }
 
+        if (!isAccepted(itemStream.getName())) {
+            LOG.warn("File field [{}] rejected", itemStream.getName());

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AZPy_bDOEosXzZdCgNCG-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AZPy_bDOEosXzZdCgNCG&open=AZPy_bDOEosXzZdCgNCG&pullRequest=1156";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/733)



##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java:
##########
@@ -113,6 +113,16 @@
     protected void processFileField(FileItem item) {
         LOG.debug("Item is a file upload");
 
+        if (!isAccepted(item.getName())) {
+            LOG.warn("File name [{}] is not accepted", item.getName());

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AZPy_bBpEosXzZdCgNBu-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AZPy_bBpEosXzZdCgNBu&open=AZPy_bBpEosXzZdCgNBu&pullRequest=1156";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/728)



##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java:
##########
@@ -134,6 +144,11 @@
         try {
             LOG.debug("Item is a normal form field");
 
+            if (!isAccepted(item.getFieldName())) {
+                LOG.warn("Form field name [{}] is not accepted", 
item.getFieldName());

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AZPy_bBpEosXzZdCgNBw-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AZPy_bBpEosXzZdCgNBw&open=AZPy_bBpEosXzZdCgNBw&pullRequest=1156";>SonarQube
 Cloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/730)





Issue Time Tracking
-------------------

    Worklog Id:     (was: 949739)
    Time Spent: 0.5h  (was: 20m)

> Exclude not malicious names
> ---------------------------
>
>                 Key: WW-5501
>                 URL: https://issues.apache.org/jira/browse/WW-5501
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Lukasz Lenart
>            Priority: Major
>             Fix For: 6.7.1, 7.1.0
>
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)