This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya 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 8114f9fb6a Fix webtools ProgramExport, Cache Maintenance and Thread
List; normalise the ProgramExport pattern list (#1845)
8114f9fb6a is described below
commit 8114f9fb6a7232861d1ac0386063357d4f4881df
Author: Krishna Uprit <[email protected]>
AuthorDate: Fri Sep 4 10:38:32 2026 +0530
Fix webtools ProgramExport, Cache Maintenance and Thread List; normalise
the ProgramExport pattern list (#1845)
Small fixes around webtools/ProgramExport and the shared Groovy class
loader.
- GroovyUtil: the compile-time AST restrictions meant for inline
${groovy:...}
scriptlets were also being applied to component:// .groovy files, so
screen
scripts that reference java.lang.Thread / java.lang.Runtime —
ProgramExport,
Cache Maintenance (FindUtilCache) and Thread List (Threads) — stopped
compiling and returned HTTP 500. The file-location path now uses its own
GroovyClassLoader; the inline path is unchanged (GroovyUtilTests still
pass).
- ProgramExport: the in-file SecureASTCustomizer combined an empty
static-imports allow-list with indirect import checking and rejected
every
method call, even the built-in example. Switched imports/static imports
to
disallowed-list mode, restored java.lang.Object to the allowed
receivers,
and updated the deprecated setter names.
- ProgramExport: several pattern-list entries matched only the plain
obj.name(
spelling; they now also match Groovy's quoted obj."name"() form,
consistently
across the list.
---------
Co-authored-by: Krishnauprit18 <[email protected]>
---
.../org/apache/ofbiz/base/util/GroovyUtil.java | 38 ++++---
.../ofbiz/webtools/entity/ProgramExport.groovy | 118 +++++++++++++--------
2 files changed, 98 insertions(+), 58 deletions(-)
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java
index 51aec9a1f2..eff00da529 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java
@@ -49,22 +49,35 @@ public final class GroovyUtil {
private static final String MODULE = GroovyUtil.class.getName();
private static final UtilCache<String, Class<?>> PARSED_SCRIPTS =
UtilCache.createUtilCache("script.GroovyLocationParsedCache", 0, 0, false);
private static final GroovyClassLoader GROOVY_CLASS_LOADER;
+ private static final GroovyClassLoader GROOVY_CLASS_LOADER_FOR_LOCATION;
private static final CompilerConfiguration SANDBOXED_COMPILER_CONFIG;
private GroovyUtil() { }
static {
- GroovyClassLoader groovyClassLoader = null;
+ GroovyClassLoader sandboxedClassLoader = null;
+ GroovyClassLoader locationClassLoader = null;
String scriptBaseClass = UtilProperties.getPropertyValue("groovy",
"scriptBaseClass");
if (!scriptBaseClass.isEmpty()) {
- CompilerConfiguration conf = new CompilerConfiguration();
- conf.setScriptBaseClass(scriptBaseClass);
- // Same compile-time AST restrictions as SANDBOXED_COMPILER_CONFIG
below, so that parseClass(),
- // not just eval(), refuses OS-execution APIs and dynamic
class-loading.
- conf.addCompilationCustomizers(buildSecureAstCustomizer());
- groovyClassLoader = new
GroovyClassLoader(GroovyUtil.class.getClassLoader(), conf);
+ // Used by parseClass(String): compiles ${groovy:...} substrings
and inline <script> bodies passed to
+ // ScriptUtil.parseScript(), which can carry caller-supplied text.
Keep the compile-time
+ // SecureASTCustomizer here as an injection backstop (see
GroovyUtilTests).
+ CompilerConfiguration sandboxedConf = new CompilerConfiguration();
+ sandboxedConf.setScriptBaseClass(scriptBaseClass);
+
sandboxedConf.addCompilationCustomizers(buildSecureAstCustomizer());
+ sandboxedClassLoader = new
GroovyClassLoader(GroovyUtil.class.getClassLoader(), sandboxedConf);
+
+ // Used by parseClass(InputStream, location): compiles *.groovy
files resolved from a component://
+ // location (controller events, service implementations, screen
scripts). These are first-party
+ // files shipped with the framework/plugins, never request input,
so they are intentionally NOT
+ // run through SecureASTCustomizer - some legitimately use
java.lang.Thread / java.lang.Runtime
+ // (e.g. the webtools Threads and Cache Maintenance screens) and
must still compile.
+ CompilerConfiguration locationConf = new CompilerConfiguration();
+ locationConf.setScriptBaseClass(scriptBaseClass);
+ locationClassLoader = new
GroovyClassLoader(GroovyUtil.class.getClassLoader(), locationConf);
}
- GROOVY_CLASS_LOADER = groovyClassLoader;
+ GROOVY_CLASS_LOADER = sandboxedClassLoader;
+ GROOVY_CLASS_LOADER_FOR_LOCATION = locationClassLoader;
}
static {
@@ -73,8 +86,9 @@ public final class GroovyUtil {
}
/**
- * Builds a fresh {@link SecureASTCustomizer} applying the compile-time
AST restrictions used by both
- * GROOVY_CLASS_LOADER and SANDBOXED_COMPILER_CONFIG. Blocks OS-execution
APIs and dynamic class-loading
+ * Builds a fresh {@link SecureASTCustomizer} applying the compile-time
AST restrictions used by
+ * GROOVY_CLASS_LOADER (parseClass(String)) and SANDBOXED_COMPILER_CONFIG
(eval()), but not by
+ * GROOVY_CLASS_LOADER_FOR_LOCATION (trusted component:// *.groovy files).
Blocks OS-execution APIs and dynamic class-loading
* as a defence-in-depth measure.
* <p>Returns a new instance on every call rather than a shared constant:
{@code SecureASTCustomizer}
* visits the AST during compilation, so handing the same instance to two
{@code CompilerConfiguration}s
@@ -231,8 +245,8 @@ public final class GroovyUtil {
*/
private static Class<?> parseClass(InputStream in, String location) throws
IOException {
String classText = UtilIO.readString(in);
- if (GROOVY_CLASS_LOADER != null) {
- return GROOVY_CLASS_LOADER.parseClass(classText, location);
+ if (GROOVY_CLASS_LOADER_FOR_LOCATION != null) {
+ return GROOVY_CLASS_LOADER_FOR_LOCATION.parseClass(classText,
location);
} else {
try (GroovyClassLoader classLoader = new GroovyClassLoader()) {
return classLoader.parseClass(classText, location);
diff --git
a/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/entity/ProgramExport.groovy
b/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/entity/ProgramExport.groovy
index de7b415b11..204ac89c83 100644
---
a/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/entity/ProgramExport.groovy
+++
b/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/entity/ProgramExport.groovy
@@ -77,23 +77,27 @@ if (security.hasPermission('ENTITY_MAINT', session)) {
// (?s) flag for multi-line/dotall matching to prevent whitespace bypass
List<String> dangerousPatterns = [
// Process & Command Execution + Runtime Variants
- /(?s)Runtime\s*\.\s*getRuntime\s*\(\s*\)/,
+ // NOTE: patterns that match a member access ('.name(',
'Type.name', '.name') carry an
+ // optional ['"]? around the name so they also match Groovy's
quoted-member syntax
+ // (obj."name"(), Type."name"()) - a normal language feature that
otherwise bypasses a
+ // literal-text match.
+ /(?s)Runtime\s*\.\s*['"]?getRuntime['"]?\s*\(\s*\)/,
/(?s)['"]java\.lang\.Runtime['"]\.class/,
- /(?s)Runtime\.class\.getDeclaredMethod/,
- /(?s)getRuntime\s*\(\s*\)\.exec/,
+ /(?s)Runtime\s*\.\s*class\s*\.\s*['"]?getDeclaredMethod['"]?/,
+ /(?s)getRuntime\s*\(\s*\)\.\s*['"]?exec['"]?/,
/(?s)ProcessBuilder/,
- /(?s)\.\s*execute\s*\(/,
- /(?s)System\s*\.\s*exit/,
+ /(?s)\.\s*['"]?execute['"]?\s*\(/,
+ /(?s)System\s*\.\s*['"]?exit['"]?/,
// Reflection & ClassLoading
- /(?s)Class\s*\.\s*forName/,
- /(?s)\.newInstance\s*\(/,
- /(?s)\.getDeclaredMethod/,
- /(?s)\.getDeclaredField/,
- /(?s)\.getMethod\s*\(/,
- /(?s)\.getField\s*\(/,
- /(?s)\.invoke\s*\(/,
- /(?s)\.loadClass\s*\(/,
- /(?s)\.getClassLoader\s*\(/,
+ /(?s)Class\s*\.\s*['"]?forName['"]?/,
+ /(?s)\.\s*['"]?newInstance['"]?\s*\(/,
+ /(?s)\.\s*['"]?getDeclaredMethod['"]?/,
+ /(?s)\.\s*['"]?getDeclaredField['"]?/,
+ /(?s)\.\s*['"]?getMethod['"]?\s*\(/,
+ /(?s)\.\s*['"]?getField['"]?\s*\(/,
+ /(?s)\.\s*['"]?invoke['"]?\s*\(/,
+ /(?s)\.\s*['"]?loadClass['"]?\s*\(/,
+ /(?s)\.\s*['"]?getClassLoader['"]?\s*\(/,
/(?s)java\s*\.\s*lang\s*\.\s*reflect/,
/(?s)URLClassLoader/,
/(?s)GroovyClassLoader/,
@@ -101,21 +105,21 @@ if (security.hasPermission('ENTITY_MAINT', session)) {
/(?s)javax\s*\.\s*script/,
/(?s)sun\s*\.\s*misc\s*\.\s*Unsafe/,
// Eval/GroovyShell Blocking
- /(?s)Eval\s*\.\s*me/,
- /(?s)Eval\s*\.\s*x/,
- /(?s)Eval\s*\.\s*xy/,
- /(?s)Eval\s*\.\s*xyz/,
+ /(?s)Eval\s*\.\s*['"]?me['"]?/,
+ /(?s)Eval\s*\.\s*['"]?x['"]?/,
+ /(?s)Eval\s*\.\s*['"]?xy['"]?/,
+ /(?s)Eval\s*\.\s*['"]?xyz['"]?/,
/(?s)GroovyShell/,
- /(?s)\.evaluate\s*\(/,
+ /(?s)\.\s*['"]?evaluate['"]?\s*\(/,
// File System Operations
/(?s)java\s*\.\s*io\s*\.\s*File\s*\(/,
/(?s)new\s+File\s*\(/,
- /(?s)Files\s*\.\s*readAllBytes/,
- /(?s)Paths\s*\.\s*get/,
- /(?s)\.toFile\s*\(/,
- /(?s)\.getResourceAsStream\s*\(/,
- /(?s)\.getText\s*\(/,
- /(?s)\.bytes\b/,
+ /(?s)Files\s*\.\s*['"]?readAllBytes['"]?/,
+ /(?s)Paths\s*\.\s*['"]?get['"]?/,
+ /(?s)\.\s*['"]?toFile['"]?\s*\(/,
+ /(?s)\.\s*['"]?getResourceAsStream['"]?\s*\(/,
+ /(?s)\.\s*['"]?getText['"]?\s*\(/,
+ /(?s)\.\s*(?:bytes\b|['"]bytes['"])/,
// Network Operations
/(?s)Socket\s*\(/,
/(?s)ServerSocket/,
@@ -125,8 +129,8 @@ if (security.hasPermission('ENTITY_MAINT', session)) {
/(?s)java\s*\.\s*net\s*\./,
/(?s)URL\s*\(/,
/(?s)NetworkInterface/,
- /(?s)\.openConnection\s*\(/,
- /(?s)\.connect\s*\(/,
+ /(?s)\.\s*['"]?openConnection['"]?\s*\(/,
+ /(?s)\.\s*['"]?connect['"]?\s*\(/,
// OFBiz Multitenancy Bypass
/(?s)DelegatorFactory/
]
@@ -141,24 +145,45 @@ if (security.hasPermission('ENTITY_MAINT', session)) {
// Groovy Sandbox with SecureASTCustomizer
SecureASTCustomizer secureCustomizer = new SecureASTCustomizer()
secureCustomizer.with {
- // Import whitelist - only safe OFBiz entity classes
- setImportsWhitelist([
- 'org.apache.ofbiz.entity.GenericValue',
- 'org.apache.ofbiz.entity.model.ModelEntity',
- 'org.apache.ofbiz.entity.condition.EntityCondition',
- 'org.apache.ofbiz.entity.condition.EntityOperator',
- 'org.apache.ofbiz.entity.util.EntityQuery',
- 'org.apache.ofbiz.entity.util.EntityFindOptions',
- 'java.util.List',
- 'java.util.Map',
- 'java.util.Set'
+ // Imports and static imports both use disallowed-list (blocklist)
mode instead of an
+ // allowed-list. setIndirectImportCheckEnabled(true) below makes
SecureASTCustomizer check
+ // the *receiver type* of every method call, constructor call, and
method pointer expression
+ // in the script against this same imports configuration - not just
against explicit import
+ // statements (which the script can never write anyway, since
KEYWORD_IMPORT is disallowed
+ // below). An allowed-list here would therefore have to include every
legitimate type ever
+ // used as a receiver (String, Delegator, EntityFindOptions, etc.),
and a narrow allowed-list
+ // combined with indirect checks rejects every expression in the
script, not just imports.
+ // Actual receiver-type restriction for method calls is still tightly
enforced separately by
+ // setAllowedReceivers below; this blocklist only needs to stop
indirect FQCN construction
+ // (e.g. "new java.lang.ProcessBuilder(...)") of genuinely dangerous
classes, matching the
+ // pattern already used in org.apache.ofbiz.base.util.GroovyUtil's
eval() sandbox.
+ // SecureASTCustomizer throws IllegalArgumentException if both an
allowed list and a
+ // disallowed list are set for the same allowed/disallowed pair, so
only the disallowed
+ // setters are used for imports, star imports, static imports and
static star imports here -
+ // do not add any setAllowed(Static)(Star)Imports call alongside these.
+ setDisallowedImports([
+ 'java.lang.Runtime',
+ 'java.lang.ProcessBuilder',
+ 'java.lang.ClassLoader',
+ 'java.lang.Thread',
+ 'java.lang.reflect.Method',
+ 'java.lang.reflect.Field',
+ 'java.net.Socket',
+ 'java.net.ServerSocket',
+ 'groovy.lang.GroovyShell',
+ 'groovy.lang.GroovyClassLoader'
+ ])
+ setDisallowedStarImports([])
+ setDisallowedStaticImports([
+ 'java.lang.Runtime.getRuntime',
+ 'java.lang.Runtime.exec',
+ 'java.lang.System.exit',
+ 'java.lang.Class.forName'
])
- setStarImportsWhitelist([])
- setStaticImportsWhitelist([])
- setStaticStarImportsWhitelist([])
+ setDisallowedStaticStarImports([])
setIndirectImportCheckEnabled(true)
// Constant types whitelist
- setConstantTypesClassesWhiteList([
+ setAllowedConstantTypesClasses([
Object, String, Integer, Long, Float, Double, Boolean,
Integer.TYPE, Long.TYPE, Float.TYPE, Double.TYPE, Boolean.TYPE,
BigDecimal, BigInteger,
@@ -170,14 +195,15 @@ if (security.hasPermission('ENTITY_MAINT', session)) {
List, Map, Set
])
// Token and statement restrictions
- setTokensBlacklist([KEYWORD_PACKAGE, KEYWORD_IMPORT])
- setStatementsBlacklist([
+ setDisallowedTokens([KEYWORD_PACKAGE, KEYWORD_IMPORT])
+ setDisallowedStatements([
WhileStatement, ForStatement,
SwitchStatement
])
- setExpressionsBlacklist([MethodPointerExpression])
+ setDisallowedExpressions([MethodPointerExpression])
// Receiver whitelist - only safe OFBiz entity operations
- setReceiversWhiteList([
+ setAllowedReceivers([
+ 'java.lang.Object',
'org.apache.ofbiz.entity.Delegator',
'org.apache.ofbiz.entity.util.EntityQuery',
'org.apache.ofbiz.entity.util.EntityFindOptions',
