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 fa45815a6d Fixed: CMS use-when expressions still templated into string
literals (#1750)
fa45815a6d is described below
commit fa45815a6ddb9fb3f47420b3c95c552f0eb90e98
Author: Krishna Uprit <[email protected]>
AuthorDate: Thu Aug 27 17:05:33 2026 +0530
Fixed: CMS use-when expressions still templated into string literals (#1750)
Follow-up to
[d645c9096d](https://github.com/ashishvijaywargiya/ofbiz-framework/commit/d645c9096dad73e2876e41ee0368684f3c41cff2)
(fix/cms-usewhen-groovy-eval-verify).
- caContentAssocTypeId's use-when attributes in CMSForms.xml templated
the value into a quoted string literal before evaluation, the same
pattern already rewritten for caContentIdTo/caMapKey in the same form.
- Rewrote caContentAssocTypeId to reference the context variable
directly, matching the idiom used elsewhere.
- Also converted caFromDate, caThruDate, contentId, and
currentValue.dataResourceId in the same form to the same idiom for
consistency.
---------
Thank you Krishna Uprit for the contribution.
Co-authored-by: Ashish Vijaywargiya <[email protected]>
Co-authored-by: Krishnauprit18 <[email protected]>
---
applications/content/widget/cms/CMSForms.xml | 26 ++++----
.../ofbiz/widget/model/ModelFormFieldTest.java | 75 ++++++++++++++++++++++
2 files changed, 88 insertions(+), 13 deletions(-)
diff --git a/applications/content/widget/cms/CMSForms.xml
b/applications/content/widget/cms/CMSForms.xml
index 9fe93a72cc..1ca1d76f2d 100644
--- a/applications/content/widget/cms/CMSForms.xml
+++ b/applications/content/widget/cms/CMSForms.xml
@@ -377,27 +377,27 @@ under the License.
<display description=""/>
</field>
<field name="caContentIdTo" position="1"
- use-when=""${caContentIdTo}".length()>0" >
+ use-when="caContentIdTo!=null&&caContentIdTo.length()>0" >
<display />
</field>
<field name="caContentIdTo" position="1"
- use-when=""${caContentIdTo}".length()==0" >
+ use-when="caContentIdTo==null||caContentIdTo.length()==0" >
<text />
</field>
<field name="caMapKey" position="1"
- use-when=""${caMapKey}".length()==0" >
+ use-when="caMapKey==null||caMapKey.length()==0" >
<text />
</field>
<field name="caMapKey" position="1"
- use-when=""${caMapKey}".length()>0" >
+ use-when="caMapKey!=null&&caMapKey.length()>0" >
<text />
</field>
<field name="caContentAssocTypeId" position="1"
- use-when=""${caContentAssocTypeId}".length()>0" >
+
use-when="caContentAssocTypeId!=null&&caContentAssocTypeId.length()>0" >
<display />
</field>
<field name="caContentAssocTypeId" position="1"
- use-when=""${caContentAssocTypeId}".length()==0" >
+
use-when="caContentAssocTypeId==null||caContentAssocTypeId.length()==0" >
<drop-down allow-empty="true">
<entity-options entity-name="ContentAssocType"
key-field-name="contentAssocTypeId"/>
</drop-down>
@@ -407,28 +407,28 @@ under the License.
<entity-options entity-name="MetaDataPredicate"
key-field-name="metaDataPredicateId"/>
</drop-down>
</field>
- <field name="caFromDate" title="${uiLabelMap.CommonFromDate}"
widget-style="buttontext" position="1"
use-when=""${caFromDate}".length()>0">
+ <field name="caFromDate" title="${uiLabelMap.CommonFromDate}"
widget-style="buttontext" position="1" use-when="caFromDate!=null">
<display default-value="${nowTimestamp}"/>
</field>
- <field name="caFromDate" title="${uiLabelMap.CommonFromDate}"
widget-style="buttontext" position="1"
use-when=""${caFromDate}".length()==0">
+ <field name="caFromDate" title="${uiLabelMap.CommonFromDate}"
widget-style="buttontext" position="1" use-when="caFromDate==null">
<date-time default-value="${nowTimestamp}"/>
</field>
- <field name="caThruDate" title="${uiLabelMap.CommonThruDate}"
widget-style="buttontext" position="1"
use-when=""${caThruDate}".length()>0">
+ <field name="caThruDate" title="${uiLabelMap.CommonThruDate}"
widget-style="buttontext" position="1" use-when="caThruDate!=null">
<date-time/>
</field>
- <field name="caThruDate" title="${uiLabelMap.CommonThruDate}"
widget-style="buttontext" position="1"
use-when=""${caThruDate}".length()==0">
+ <field name="caThruDate" title="${uiLabelMap.CommonThruDate}"
widget-style="buttontext" position="1" use-when="caThruDate==null">
<date-time/>
</field>
<field name="contentTitle" title="${uiLabelMap.ContentContent}"
title-style="h1" map-name="dummy">
<display description=""/>
</field>
<field name="contentId" position="1"
- use-when=""${currentValue.contentId}".length()>0" >
+
use-when="currentValue.contentId!=null&&currentValue.contentId.length()>0"
>
<display />
</field>
<field name="contentId" position="1"
- use-when=""${currentValue.contentId}".length()==0" >
+
use-when="currentValue.contentId==null||currentValue.contentId.length()==0" >
<text />
</field>
<field name="templateDataResourceId">
@@ -483,7 +483,7 @@ under the License.
</field>
<field name="dataResourceId"
title="${uiLabelMap.ContentDataResourceId}">
<lookup target-form-name="LookupDataResource">
- <sub-hyperlink
use-when=""${currentValue.dataResourceId}".length()>0"
link-style="buttontext" target-type="intra-app" target="gotoDataResource"
description="${uiLabelMap.ContentGoToDataResource}">
+ <sub-hyperlink
use-when="currentValue.dataResourceId!=null&&currentValue.dataResourceId.length()>0"
link-style="buttontext" target-type="intra-app" target="gotoDataResource"
description="${uiLabelMap.ContentGoToDataResource}">
<parameter param-name="dataResourceId"
from-field="currentValue.dataResourceId"/>
</sub-hyperlink>
<!--
diff --git
a/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
b/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
index 1b98cabb90..a3a50189c9 100644
---
a/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
+++
b/framework/widget/src/test/java/org/apache/ofbiz/widget/model/ModelFormFieldTest.java
@@ -22,6 +22,8 @@ import static
org.apache.ofbiz.widget.model.ModelFormField.from;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;
import java.util.Arrays;
@@ -158,4 +160,77 @@ public final class ModelFormFieldTest {
ImmutableMap.of("prefix", "P1", "key1", "AA,BB , CC"));
assertThat(targetParameterList, Matchers.contains("P1TargetParam",
"AA", "BB", "CC"));
}
+
+ /**
+ * Regression test for the CMS caContentIdTo/caMapKey use-when handling
(applications/content/widget/cms/CMSForms.xml).
+ * <p>{@code shouldUse} expands the {@code use-when} attribute with {@link
org.apache.ofbiz.base.util.string.FlexibleStringExpander}
+ * before handing it to Groovy: a {@code
use-when=""${var}".length()>0"} template textually splices the
+ * <em>stringified value</em> of a context variable into the source string
that is then compiled and executed. If that
+ * context variable is populated from an HTTP request parameter, a value
containing a closing quote can escape the
+ * string literal and reach the surrounding Groovy source, which -- as
demonstrated separately via reflection
+ * (Class.forName("java.lang.Runtime") + Method.invoke) -- is not stopped
by GroovyUtil's compile-time
+ * SecureASTCustomizer sandbox. The fix (applied after this test is
written) is to stop templating the value into a
+ * string literal and instead reference the context variable directly, the
same safe idiom already used by the vast
+ * majority of use-when expressions elsewhere in the codebase (e.g.
"currentValue==null").
+ */
+ @Test
+ public void useWhenDoesNotExecuteContextValueAsGroovyCode() throws
Exception {
+ // A synchronous, in-JVM side effect (no OS process spawn) so the test
is deterministic: no race waiting
+ // on an async subprocess to finish writing a file. Running arbitrary
code is proof enough here; a
+ // separate, manually-verified reproduction additionally shows the
same string literal escape reaching
+ // Runtime.exec() via reflection (Class.forName("java.lang.Runtime") +
Method.invoke), bypassing
+ // GroovyUtil's compile-time SecureASTCustomizer sandbox -- that
sandbox is irrelevant to *this* test,
+ // since the sandbox was never the defense that mattered: nothing
should be textually splicing untrusted
+ // data into compiled source.
+ String marker = "ofbiz.usewhen.test.marker";
+ System.clearProperty(marker);
+ try {
+ // Matches the fixed use-when now in CMSForms.xml: a direct,
unquoted binding reference instead of a
+ // FlexibleStringExpander "${var}" template spliced into a string
literal.
+ ModelFormField field = from(b ->
b.setName("caContentIdTo").setUseWhen("caContentIdTo!=null&&caContentIdTo.length()>0"));
+
+ HashMap<String, Object> legitimate = new HashMap<>();
+ legitimate.put("caContentIdTo", "LEGITIMATE_ID");
+ assertTrue(field.shouldUse(legitimate), "A non-empty legitimate
value must still satisfy the use-when");
+
+ HashMap<String, Object> empty = new HashMap<>();
+ assertFalse(field.shouldUse(empty), "A missing value must not
satisfy the use-when");
+
+ HashMap<String, Object> untrusted = new HashMap<>();
+ untrusted.put("caContentIdTo", "x\";System.setProperty(\"" +
marker + "\",\"SIDE_EFFECT\");\"x");
+ // The untrusted text is still non-empty, so the field is
legitimately shown -- that part
+ // is expected and harmless. What must NOT happen is the embedded
statement executing as code.
+ assertTrue(field.shouldUse(untrusted), "The untrusted value is
still non-empty text, so use-when legitimately evaluates true");
+ assertFalse(System.getProperty(marker) != null, "The embedded
Groovy statement must not execute as code");
+ } finally {
+ System.clearProperty(marker);
+ }
+ }
+
+ /**
+ * Same handling, same fix, for the sibling caMapKey use-when in the same
CMSForms.xml form.
+ * See {@link #useWhenDoesNotExecuteContextValueAsGroovyCode()} for the
full explanation.
+ */
+ @Test
+ public void useWhenDoesNotExecuteContextValueAsGroovyCodeForCaMapKey()
throws Exception {
+ String marker = "ofbiz.usewhen.test.marker.camapkey";
+ System.clearProperty(marker);
+ try {
+ ModelFormField field = from(b ->
b.setName("caMapKey").setUseWhen("caMapKey!=null&&caMapKey.length()>0"));
+
+ HashMap<String, Object> legitimate = new HashMap<>();
+ legitimate.put("caMapKey", "LEGITIMATE_KEY");
+ assertTrue(field.shouldUse(legitimate), "A non-empty legitimate
value must still satisfy the use-when");
+
+ HashMap<String, Object> empty = new HashMap<>();
+ assertFalse(field.shouldUse(empty), "A missing value must not
satisfy the use-when");
+
+ HashMap<String, Object> untrusted = new HashMap<>();
+ untrusted.put("caMapKey", "x\";System.setProperty(\"" + marker +
"\",\"SIDE_EFFECT\");\"x");
+ assertTrue(field.shouldUse(untrusted), "The untrusted value is
still non-empty text, so use-when legitimately evaluates true");
+ assertFalse(System.getProperty(marker) != null, "The embedded
Groovy statement must not execute as code");
+ } finally {
+ System.clearProperty(marker);
+ }
+ }
}
