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
The following commit(s) were added to refs/heads/trunk by this push:
new 9e89930 Fixed: Fix OFBiz speficic Javascript securiy issues reported
by GH CodeQL (OFBIZ-12366)
9e89930 is described below
commit 9e89930e00111994806bdcb467888bc30123f17e
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Wed Nov 10 16:08:02 2021 +0100
Fixed: Fix OFBiz speficic Javascript securiy issues reported by GH CodeQL
(OFBIZ-12366)
Previously in commit dfc7ee4 I blindly followed GH CodeQL's advice about js
CSS.
I had a doubt, indeed it was irrelevant. I misinterpreted the advice.
Then I tried to follow OWASP's advice about using node-esapi but was unable
to
make it work in a reasonable time. Neither with "node require" nor
requireJS;
ie using either CommonJS or AMD format, I guess I missed something there.
Anyway, I finally I set my sights on DOMPurify which is doing the job.
There is only one "foot-gun potential":
https://github.com/cure53/DOMPurify#user-content-is-there-any-foot-gun-potential
"If you first sanitize HTML and then modify it afterwards, you might easily
void
the effects of sanitization. If you feed the sanitized markup to another
library
after sanitization, please be certain that the library doesn't mess around
with
the HTML on its own."
Fortunately, that's not a concern in our simple case.
This commit
* fixes the 3 remaining js vulnerabilty reported by
* prevents checking all under node_modules since it's done by npm in console
(a bit weak though, ie not clearly on purpose)
* gives more information about 'ci' vs 'install' in npmInstallCommand
---
.github/workflows/codeql-analysis.yml | 7 ++++---
build.gradle | 6 ++++--
themes/common-theme/webapp/common/js/package-lock.json | 5 +++++
themes/common-theme/webapp/common/js/package.json | 3 ++-
themes/common-theme/webapp/common/js/util/OfbizUtil.js | 4 ++--
themes/common-theme/webapp/common/js/util/fieldlookup.js | 2 +-
themes/common-theme/widget/CommonScreens.xml | 2 ++
themes/common-theme/widget/Theme.xml | 1 +
8 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/.github/workflows/codeql-analysis.yml
b/.github/workflows/codeql-analysis.yml
index d33f557..09edc20 100644
--- a/.github/workflows/codeql-analysis.yml
+++ b/.github/workflows/codeql-analysis.yml
@@ -29,15 +29,16 @@ on:
paths:
- '**.java'
- '**.js'
- #paths-ignore:
- # - src/node_modules
- # - '**/*.test.js'
+ paths-ignore:
+ - **/node_modules/**
pull_request:
# The branches below must be a subset of the branches above
branches: [ trunk ]
paths:
- '**.java'
- '**.js'
+ paths-ignore:
+ - **/node_modules/**
schedule:
- cron: '27 15 * * 1'
diff --git a/build.gradle b/build.gradle
index d5336c2..58e0566 100644
--- a/build.gradle
+++ b/build.gradle
@@ -110,11 +110,13 @@ javadoc {
node {
download = true
version = "12.18.1"
- // npmVersion will be the one that comes default with the node
+ // npmVersion will be the one that comes default with 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
+ // Set the work directory where node_modules should be located
nodeModulesDir =
file("${project.projectDir}/themes/common-theme/webapp/common/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'
}
diff --git a/themes/common-theme/webapp/common/js/package-lock.json
b/themes/common-theme/webapp/common/js/package-lock.json
index 703ffaf..59c754f 100644
--- a/themes/common-theme/webapp/common/js/package-lock.json
+++ b/themes/common-theme/webapp/common/js/package-lock.json
@@ -3,6 +3,11 @@
"requires": true,
"lockfileVersion": 1,
"dependencies": {
+ "dompurify": {
+ "version": "2.3.3",
+ "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-2.3.3.tgz";,
+ "integrity":
"sha512-dqnqRkPMAjOZE0FogZ+ceJNM2dZ3V/yNOuFB7+39qpO93hHhfRpHw3heYQC7DPK9FqbQTfBKUJhiSfz4MvXYwg=="
+ },
"jquery": {
"version": "3.5.1",
"resolved": "https://registry.npmjs.org/jquery/-/jquery-3.5.1.tgz";,
diff --git a/themes/common-theme/webapp/common/js/package.json
b/themes/common-theme/webapp/common/js/package.json
index 7615d5a..ac5685e 100644
--- a/themes/common-theme/webapp/common/js/package.json
+++ b/themes/common-theme/webapp/common/js/package.json
@@ -7,6 +7,7 @@
"jquery": "^3.5.1",
"jquery-migrate": "^3.3.1",
"jquery-validation": "^1.19.3",
- "jquery.browser": "^0.1.0"
+ "jquery.browser": "^0.1.0",
+ "dompurify": "^2.3.3"
}
}
diff --git a/themes/common-theme/webapp/common/js/util/OfbizUtil.js
b/themes/common-theme/webapp/common/js/util/OfbizUtil.js
index cb8d2a9..41e11bd 100644
--- a/themes/common-theme/webapp/common/js/util/OfbizUtil.js
+++ b/themes/common-theme/webapp/common/js/util/OfbizUtil.js
@@ -879,7 +879,7 @@ function ajaxAutoCompleter(areaCsvString, showDescription,
defaultMinLength, def
var queryArgs = {"term": request.term};
if (typeof args == "object" && jQuery.isArray(args)) {
for (var i = 0; i < args.length; i++) {
- queryArgs["parm" + i] = jQuery(args[i]).val();
+ queryArgs["parm" + i] =
DOMPurify.sanitize(jQuery(args[i]).val())
}
}
jQuery.ajax({
@@ -904,7 +904,7 @@ function ajaxAutoCompleter(areaCsvString, showDescription,
defaultMinLength, def
if (typeof autocomp != 'undefined') {
jQuery.each(autocomp, function(index, item){
- item.label =
jQuery("<div>").html(item.label).text();
+ item.label =
DOMPurify.sanitize(jQuery("<div>").html(item.label).text());
})
// autocomp is the JSON Object which will be used
for the autocomplete box
response(autocomp);
diff --git a/themes/common-theme/webapp/common/js/util/fieldlookup.js
b/themes/common-theme/webapp/common/js/util/fieldlookup.js
index 5d8029c..ee24348 100644
--- a/themes/common-theme/webapp/common/js/util/fieldlookup.js
+++ b/themes/common-theme/webapp/common/js/util/fieldlookup.js
@@ -304,7 +304,7 @@ var Lookup = function(options) {
var queryArgs = "presentation=" + options.presentation;
if (typeof options.args == "object" && jQuery.isArray(options.args)) {
for ( var i = 0; i < options.args.length; i++) {
- queryArgs += "&parm" + i + "=" +
jQuery(jQuery.find(options.args[i]).val());
+ queryArgs += DOMPurify.sanitize("&parm" + i + "=" +
jQuery(options.args[i]).val());
}
}
diff --git a/themes/common-theme/widget/CommonScreens.xml
b/themes/common-theme/widget/CommonScreens.xml
index bafc502..8378bd3 100644
--- a/themes/common-theme/widget/CommonScreens.xml
+++ b/themes/common-theme/widget/CommonScreens.xml
@@ -349,6 +349,7 @@ under the License.
<set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/jquery.browser/dist/jquery.browser.min.js"
global="true"/>
<set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/jquery-migrate/dist/jquery-migrate.min.js"
global="true"/>
<set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/jquery/dist/jquery.min.js" global="true"/>
+ <set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/node_modules/dompurify/dist/purify.min.js"
global="true"/>
<set field="layoutSettings.javaScripts[]"
value="/common/js/util/OfbizUtil.js" global="true"/>
</actions>
<widgets>
@@ -454,6 +455,7 @@ under the License.
<set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/jquery.browser/dist/jquery.browser.min.js"
global="true"/>
<set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/jquery-migrate/dist/jquery-migrate.min.js"
global="true" />
<set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/jquery/dist/jquery.min.js" global="true"/>
+ <set field="layoutSettings.javaScripts[+0]"
value="/common/js/node_modules/node_modules/dompurify/dist/purify.min.js"
global="true"/>
<!-- jQuery CSSs -->
<set field="layoutSettings.styleSheets[+0]"
value="/common/js/jquery/ui/jquery-ui-1.13.0.min.css" global="true" />
diff --git a/themes/common-theme/widget/Theme.xml
b/themes/common-theme/widget/Theme.xml
index 8f536e7..8fc1cc8 100644
--- a/themes/common-theme/widget/Theme.xml
+++ b/themes/common-theme/widget/Theme.xml
@@ -62,6 +62,7 @@ under the License.
<property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/node_modules/jquery.browser/dist/jquery.browser.min.js"/>
<property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/jquery/ui/jquery-ui-1.13.0.min.js"/>
<property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/node_modules/jquery-validation/dist/jquery.validate.min.js"/>
+ <property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/node_modules/dompurify/dist/purify.min.js"/>
<property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/util/OfbizUtil.js"/>
<property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/util/fieldlookup.js"/>
<property name="VT_HDR_JAVASCRIPT['add']"
value="/common/js/plugins/date/date.timezone-min.js"/>
