You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2022/02/14 12:41:48 UTC

[ofbiz-framework] branch trunk updated (88b5c40 -> 2e0a6e4)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.


    from 88b5c40  Improved: Change common webapp to common-theme (OFBIZ-12576)
     new 719ca41  Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
     new 2e0a6e4  Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 build.gradle                                          |  4 ++--
 framework/security/config/security.properties         |  6 +++---
 .../org/apache/ofbiz/security/SecurityUtilTest.java   | 19 ++++++++-----------
 3 files changed, 13 insertions(+), 16 deletions(-)

[ofbiz-framework] 01/02: Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)

Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 719ca41467c369fc1f382c3b260812a133c863a6
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Mon Feb 14 13:16:55 2022 +0100

    Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
    
    After reviewing https://docs.oracle.com/cd/B14099_19/web.1012/b14014/jspxml.htm,
    since we use "<jsp:" this removes "scriptlet>", "declaration>" and "expression>".
    They are redundant with "<jsp:"; "<jsp:" covers those already.
    
    Also adds eval( for js and shrinks base64_decode to decode, and processbuilder
    to process because of OFBIZ 12571
    
    I always test the denied tokens against a lib of  images with 33 600 elements (
    3.99 GB). If an image contains the token it can't be in the denied list.
---
 framework/security/config/security.properties         |  6 +++---
 .../org/apache/ofbiz/security/SecurityUtilTest.java   | 19 ++++++++-----------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index 3c6ae64..ac75ee4 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -249,10 +249,10 @@ allowAllUploads=
 #-- "freemarker" should be OK, should not be used in Freemarker templates, not part of the syntax.
 #-- Else "template.utility.Execute" is a good replacement but not as much catching, who knows...
 #-- If you are sure you are safe for a token you can remove it, etc.
-deniedWebShellTokens=freemarker,<script,javascript,<body,<form,<jsp:,scriptlet>,declaration>,expression>,<c:out,taglib,<prefix,<%@ page,\
-                     %eval,@eval,runtime,import,passthru,shell_exec,assert,str_rot13,system,base64_decode,include,\
+deniedWebShellTokens=freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@ page,\
+                     %eval,@eval,eval(,runtime,import,passthru,shell_exec,assert,str_rot13,system,decode,include,page ,\
                      chmod,mkdir,fopen,fclose,new file,upload,getfilename,download,getoutputstring,readfile,\
-                     python,perl ,/perl,ruby ,/ruby,processbuilder,function,class
+                     python,perl ,/perl,ruby ,/ruby,process,function,class,InputStream
 #-- IMPORTANT: when you change things here you need to do accordingly in SecurityUtilTest::webShellTokensTesting and run "gradlew test" --
 
 #-- Popup last-visited time from database after user has logged in.
diff --git a/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
index 9757733..2659de1 100644
--- a/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
+++ b/framework/security/src/test/java/org/apache/ofbiz/security/SecurityUtilTest.java
@@ -59,10 +59,10 @@ public class SecurityUtilTest {
     @Test
     public void webShellTokensTesting() {
         // Currently used
-        // freemarker,<script,javascript,<body,<form,<jsp:,scriptlet>,declaration>,expression>,<c:out,taglib,<prefix,<%@ page
-        // %eval,@eval,runtime,import,passthru,shell_exec,assert,str_rot13,system,base64_decode,include
-        // chmod,mkdir,fopen,fclose,new file,upload,getfilename,download,getoutputstring,readfile
-        // python,perl ,/perl,ruby ,/ruby,processbuilder,function,class
+        // freemarker,<script,javascript,<body,<form,<jsp:,<c:out,taglib,<prefix,<%@ page,\
+        // %eval,@eval,eval(,runtime,import,passthru,shell_exec,assert,str_rot13,system,decode,include,page ,\
+        // chmod,mkdir,fopen,fclose,new file,upload,getfilename,download,getoutputstring,readfile,\
+        // python,perl ,/perl,ruby ,/ruby,process,function,class
 
         try {
             List<String> allowed = new ArrayList<>();
@@ -77,9 +77,6 @@ public class SecurityUtilTest {
             assertFalse(SecuredUpload.isValidText("<body", allowed));
             assertFalse(SecuredUpload.isValidText("<form", allowed));
             assertFalse(SecuredUpload.isValidText("<jsp:", allowed));
-            assertFalse(SecuredUpload.isValidText("scriptlet>", allowed));
-            assertFalse(SecuredUpload.isValidText("declaration>", allowed));
-            assertFalse(SecuredUpload.isValidText("expression>", allowed));
             assertFalse(SecuredUpload.isValidText("<c:out", allowed));
             assertFalse(SecuredUpload.isValidText("taglib", allowed));
             assertFalse(SecuredUpload.isValidText("<prefix", allowed));
@@ -94,7 +91,7 @@ public class SecurityUtilTest {
             assertFalse(SecuredUpload.isValidText("assert", allowed));
             assertFalse(SecuredUpload.isValidText("str_rot13", allowed));
             assertFalse(SecuredUpload.isValidText("system", allowed));
-            assertFalse(SecuredUpload.isValidText("base64_decode", allowed));
+            assertFalse(SecuredUpload.isValidText("decode", allowed));
             assertFalse(SecuredUpload.isValidText("include", allowed));
 
             assertFalse(SecuredUpload.isValidText("chmod", allowed));
@@ -113,9 +110,9 @@ public class SecurityUtilTest {
             assertFalse(SecuredUpload.isValidText("/perl", allowed));
             assertFalse(SecuredUpload.isValidText("ruby ", allowed));
             assertFalse(SecuredUpload.isValidText("/ruby", allowed));
-            assertFalse(SecuredUpload.isValidText("processbuilder", allowed)); // Groovy
-            assertFalse(SecuredUpload.isValidText("function", allowed)); // Groovy
-            assertFalse(SecuredUpload.isValidText("class", allowed)); // Groovy
+            assertFalse(SecuredUpload.isValidText("process", allowed));
+            assertFalse(SecuredUpload.isValidText("function", allowed));
+            assertFalse(SecuredUpload.isValidText("class", allowed));
 
         } catch (IOException e) {
             fail(String.format("IOException occured : %s", e.getMessage()));

[ofbiz-framework] 02/02: Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)

Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 2e0a6e4f8c7eab963284160d9f04be610224f981
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Mon Feb 14 13:41:06 2022 +0100

    Improved: Create a deny list to reject webshell tokens (OFBIZ-12324)
    
    OK, as ever I did not commit build.gradle because I have to have it
    assume-unchanged with node.js version 13.14.0, I'm an outlaw
---
 build.gradle | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/build.gradle b/build.gradle
index 5cd3a71..35dc331 100644
--- a/build.gradle
+++ b/build.gradle
@@ -114,7 +114,7 @@ node {
 
     // https://github.com/node-gradle/gradle-node-plugin/blob/2.2.4/README.md
     // Set the work directory where node_modules should be located
-    nodeModulesDir = file("${project.projectDir}/themes/common-theme/webapp/common/js")
+    nodeModulesDir = file("${project.projectDir}/themes/common-theme/webapp/common-theme/js")
     // If you set a "CI" env var then ci only will be used:
     // https://github.com/node-gradle/gradle-node-plugin/blob/master/docs/faq.md#how-do-i-use-npm-ci-instead-of-npm-install
     npmInstallCommand = System.getenv("CI") ? 'ci' : 'install'
@@ -1016,7 +1016,7 @@ task cleanFooterFiles(group: cleanupGroup, description: 'clean generated footer
 }
 task cleanNpm(group: cleanupGroup, description: 'clean node modules') {
     doLast {
-        delete "${project.projectDir}/themes/common-theme/webapp/common/js/node_modules"
+        delete "${project.projectDir}/themes/common-theme/webapp/common-theme/js/node_modules"
     }
 }
 /*