This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya pushed a commit to branch release24.09
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release24.09 by this push:
new bbce2532a6 Improved input handling and access control around the
invoice email-aending flow (#1706)
bbce2532a6 is described below
commit bbce2532a6e672d08347bdab7104802efc00cf77
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Mon Aug 24 00:20:24 2026 +0530
Improved input handling and access control around the invoice email-aending
flow (#1706)
Improved input handling and access control around the invoice
email-sending flow, and adds an extra safeguard to script evaluation
used elsewhere in the framework.
No functional change is intended for normal use of this feature.
Thank you Krishna Uprit for your help.
(cherry picked from commit 1da6eab73e6ca5d09596878b96d553d81dd07f1f,
test files excluded)
---
.../accounting/servicedef/services_invoice.xml | 1 +
.../org/apache/ofbiz/base/util/GroovyUtil.java | 26 +++++++++++++++++-----
.../apache/ofbiz/common/email/EmailServices.java | 12 +++++-----
3 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/applications/accounting/servicedef/services_invoice.xml
b/applications/accounting/servicedef/services_invoice.xml
index 0bd0690dc4..54f514f3d2 100644
--- a/applications/accounting/servicedef/services_invoice.xml
+++ b/applications/accounting/servicedef/services_invoice.xml
@@ -285,6 +285,7 @@ under the License.
<service name="sendInvoicePerEmail" engine="groovy"
location="component://accounting/src/main/groovy/org/apache/ofbiz/accounting/invoice/InvoiceServicesScript.groovy"
invoke="sendInvoicePerEmail">
<description>Send an invoice per email</description>
+ <permission-service service-name="acctgInvoicePermissionCheck"
main-action="VIEW"/>
<attribute name="invoiceId" type="String" mode="IN"/>
<attribute name="sendFrom" type="String" mode="IN"/>
<attribute name="sendTo" type="String" mode="IN"/>
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 7972d8375d..d69bd62d60 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
@@ -59,16 +59,31 @@ public final class GroovyUtil {
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);
}
GROOVY_CLASS_LOADER = groovyClassLoader;
}
static {
- // Compile-time AST restrictions applied to eval() expressions.
- // Blocks OS-execution APIs and dynamic class-loading as a
defence-in-depth measure.
- // Note: SecureASTCustomizer operates at compile time and does not
constitute a
- // complete sandbox; eval() expressions should never originate from
untrusted input.
+ SANDBOXED_COMPILER_CONFIG = new CompilerConfiguration();
+
SANDBOXED_COMPILER_CONFIG.addCompilationCustomizers(buildSecureAstCustomizer());
+ }
+
+ /**
+ * 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
+ * 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
+ * used concurrently would be unsafe.
+ * <p>Note: SecureASTCustomizer operates at compile time and does not
constitute a complete sandbox;
+ * expressions compiled through either path should never originate from
untrusted input.
+ * @return a new, independently-usable SecureASTCustomizer
+ */
+ private static SecureASTCustomizer buildSecureAstCustomizer() {
SecureASTCustomizer secureAst = new SecureASTCustomizer();
secureAst.setDisallowedImports(List.of(
"java.lang.Runtime",
@@ -88,8 +103,7 @@ public final class GroovyUtil {
Thread.class,
ClassLoader.class);
secureAst.setDisallowedReceiversClasses(blockedReceivers);
- SANDBOXED_COMPILER_CONFIG = new CompilerConfiguration();
- SANDBOXED_COMPILER_CONFIG.addCompilationCustomizers(secureAst);
+ return secureAst;
}
/**
diff --git
a/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
b/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
index 32d124c201..27a8f65e57 100644
---
a/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
+++
b/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
@@ -63,7 +63,6 @@ import org.apache.ofbiz.base.util.UtilMisc;
import org.apache.ofbiz.base.util.UtilProperties;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.collections.MapStack;
-import org.apache.ofbiz.base.util.string.FlexibleStringExpander;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
@@ -537,7 +536,8 @@ public class EmailServices {
if (UtilValidate.isNotEmpty(xslfoAttachScreenLocationList)) {
List<Map<String, ? extends Object>> bodyParts = new LinkedList<>();
if (bodyText != null) {
- bodyText = FlexibleStringExpander.expandString(bodyText,
screenContext, locale);
+ // bodyText is caller-supplied (e.g. from a request
parameter); it must not be run through
+ // FlexibleStringExpander, which would compile and execute any
${groovy:...} substring it contains.
bodyParts.add(UtilMisc.<String, Object>toMap("content",
bodyText, "type", UtilValidate.isNotEmpty(contentType) ? contentType
: "text/html"));
} else {
@@ -596,7 +596,7 @@ public class EmailServices {
isMultiPart = false;
// store body and type for single part message in the context.
if (bodyText != null) {
- bodyText = FlexibleStringExpander.expandString(bodyText,
screenContext, locale);
+ // bodyText is caller-supplied; see the comment on the
identical guard above.
serviceContext.put("body", bodyText);
} else {
serviceContext.put("body", bodyWriter.toString());
@@ -611,11 +611,11 @@ public class EmailServices {
}
}
- // also expand the subject at this point, just in case it has the
FlexibleStringExpander syntax in it...
+ // subject is caller-supplied; do not run it through
FlexibleStringExpander, which would compile and
+ // execute any ${groovy:...} substring it contains -- same reasoning
as the bodyText guards above.
String subject = (String) serviceContext.remove("subject");
- subject = FlexibleStringExpander.expandString(subject, screenContext,
locale);
if (Debug.infoOn()) {
- Debug.logInfo("Expanded email subject to: " + subject, MODULE);
+ Debug.logInfo("Email subject: " + subject, MODULE);
}
serviceContext.put("subject", subject);
serviceContext.put("partyId", partyId);
