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 555f76f7e3 Fixed: arbitrary service execution via Content.serviceName
in ContentWorker (#1788)
555f76f7e3 is described below
commit 555f76f7e3679d03f4813ba90edebf57a5fd27a3
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Thu Aug 27 18:10:08 2026 +0530
Fixed: arbitrary service execution via Content.serviceName in ContentWorker
(#1788)
renderContentAsText resolved and ran whatever service name was stored in
Content.serviceName, a plain non-primary-key field writable by any
CONTENTMGR_CREATE/UPDATE account through createContent/updateContent,
using the live HTTP request parameters as the service's input. Some
content-rendering routes require no authentication, so an armed row
could be triggered anonymously.
- ContentWorker no longer reads Content.serviceName; the customMethodId
path now requires the referenced CustomMethod be typed CONTENT_RENDER
- Locked serviceName and customMethodId out of mass assignment on
createContent, updateContent, createProductContent and
updateProductContent
- Removed the now-dead serviceName field from the Catalog Manager
external content form
- Added CONTENT_RENDER as a seed CustomMethodType
- Added regression coverage for the new dispatch behavior
Thank you Krishna Uprit for your help.
---
.../content/servicedef/services_content.xml | 8 +++
.../ofbiz/content/content/ContentWorker.java | 16 ++++-
.../ofbiz/content/content/ContentTests.groovy | 79 ++++++++++++++++++++++
applications/product/servicedef/services.xml | 8 +++
.../product/widget/catalog/ProductForms.xml | 1 -
framework/common/data/CommonTypeData.xml | 3 +
6 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/applications/content/servicedef/services_content.xml
b/applications/content/servicedef/services_content.xml
index 4adcb78963..5857a4476d 100644
--- a/applications/content/servicedef/services_content.xml
+++ b/applications/content/servicedef/services_content.xml
@@ -47,6 +47,10 @@
<override name="statusId" default-value="CTNT_IN_PROGRESS"/>
<override name="contentName" allow-html="safe"/>
<override name="description" allow-html="safe"/>
+ <!-- serviceName/customMethodId choose what code renderContentAsText
runs against the live request
+ parameters; neither is set through this generic mass-assignment
path (see ContentWorker.java) -->
+ <override name="serviceName" mode="OUT"/>
+ <override name="customMethodId" mode="OUT"/>
</service>
<service name="createTextAndUploadedContent" engine="groovy" auth="true"
@@ -140,6 +144,10 @@
<!-- end of deprecated fields -->
<override name="contentName" allow-html="safe"/>
<override name="description" allow-html="safe"/>
+ <!-- serviceName/customMethodId choose what code renderContentAsText
runs against the live request
+ parameters; neither is set through this generic mass-assignment
path (see ContentWorker.java) -->
+ <override name="serviceName" mode="OUT"/>
+ <override name="customMethodId" mode="OUT"/>
</service>
<service name="updateTextContent" engine="group" auth="true">
diff --git
a/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java
b/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java
index f7895cb53a..914f51ba61 100644
---
a/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java
+++
b/applications/content/src/main/java/org/apache/ofbiz/content/content/ContentWorker.java
@@ -197,10 +197,22 @@ public class ContentWorker implements
org.apache.ofbiz.widget.content.ContentWor
// if the content has a service attached run the service
Delegator delegator = dispatcher.getDelegator();
- String serviceName = content.getString("serviceName"); //Kept for
backward compatibility
+ // NOTE: Content.serviceName is a legacy, client-writable field
(createContent/updateContent both
+ // accept it as a plain non-PK attribute) that used to be resolved and
run here directly; that let
+ // anyone able to write a Content row choose an arbitrary service to
run with the live HTTP request
+ // parameters as its input. It is no longer honored. Only a
CustomMethod explicitly typed for
+ // content rendering may be run, so arming a row requires a
CustomMethod that was deliberately set
+ // up for this purpose, not just a string dropped into the Content row
itself.
+ String serviceName = null;
GenericValue custMethod = null;
if (UtilValidate.isNotEmpty(content.getString("customMethodId"))) {
- custMethod =
EntityQuery.use(delegator).from("CustomMethod").where("customMethodId",
content.get("customMethodId")).cache().queryOne();
+ custMethod = EntityQuery.use(delegator).from("CustomMethod")
+ .where("customMethodId", content.get("customMethodId"),
"customMethodTypeId", "CONTENT_RENDER")
+ .cache().queryOne();
+ if (custMethod == null) {
+ throw new GeneralException("customMethodId [" +
content.get("customMethodId")
+ + "] on content [" + content.get("contentId") + "] is
not a content rendering method");
+ }
}
if (custMethod != null) serviceName =
custMethod.getString("customMethodName");
if (UtilValidate.isNotEmpty(serviceName)) {
diff --git
a/applications/content/src/test/groovy/org/apache/ofbiz/content/content/ContentTests.groovy
b/applications/content/src/test/groovy/org/apache/ofbiz/content/content/ContentTests.groovy
index 801f0fc75d..0296d5f38d 100644
---
a/applications/content/src/test/groovy/org/apache/ofbiz/content/content/ContentTests.groovy
+++
b/applications/content/src/test/groovy/org/apache/ofbiz/content/content/ContentTests.groovy
@@ -18,6 +18,9 @@
*******************************************************************************/
package org.apache.ofbiz.content.content
+import static org.junit.jupiter.api.Assertions.assertThrows
+
+import org.apache.ofbiz.base.util.GeneralException
import org.apache.ofbiz.base.util.UtilDateTime
import org.apache.ofbiz.entity.GenericValue
import org.apache.ofbiz.service.ServiceUtil
@@ -221,4 +224,80 @@ class ContentTests implements JupiterTestHelper {
assert serviceResult.view
}
+ // Regression coverage for the fix to renderContentAsText's service
dispatch: Content.serviceName is a
+ // legacy, client-writable field (createContent/updateContent both let a
plain content editor set it) that
+ // used to be looked up and run directly with the live HTTP request
parameters as its input. Only a
+ // CustomMethod explicitly typed CONTENT_RENDER may be run there now.
+
+ // These three build their own Content/CustomMethod fixture rows inline
(rather than relying on the
+ // testdef data-load test-case) since that data-load and this Jupiter
suite are proven, in this class,
+ // not to share read-your-writes visibility of a row committed in between
the two.
+
+ @Test
+ @Order(9)
+ void testRenderContentAsTextIgnoresServiceName() {
+ String contentId = testParams.contentId ?: 'TEST_CNT_SVCNAME'
+ String dataResourceId = testParams.dataResourceId ?: 'TEST_DR_SVCNAME'
+ delegator.create('DataResource', [dataResourceId: dataResourceId,
dataResourceTypeId: 'ELECTRONIC_TEXT'])
+ delegator.create('ElectronicText', [dataResourceId: dataResourceId,
textData: 'Test text for service name ignore check'])
+ delegator.create('Content', [contentId: contentId, contentTypeId:
'TEST_CONTENT_TYPE',
+ dataResourceId: dataResourceId, serviceName:
'aServiceThatDefinitelyDoesNotExist'])
+ GenericValue content = from('Content').where('contentId',
contentId).queryOne()
+ assert content
+ assert content.serviceName
+
+ StringWriter out = new StringWriter()
+ Map<String, Object> templateContext = [userLogin: userLogin,
requestParameters: [:]]
+ // must render the underlying data straight through, not attempt to
resolve/run the bogus service name
+ ContentWorker.renderContentAsText(dispatcher, content, out,
templateContext, Locale.US, 'text/plain', false, null)
+ assert out.toString().contains('Test text for service name ignore
check')
+ }
+
+ @Test
+ @Order(10)
+ void testRenderContentAsTextRunsTypedCustomMethod() {
+ String contentId = testParams.contentId ?: 'TEST_CNT_CM_RENDER'
+ String customMethodId = testParams.customMethodId ?: 'TEST_CM_RENDER'
+ String dataResourceId = testParams.dataResourceId ?:
'TEST_DR_CM_RENDER'
+ delegator.create('DataResource', [dataResourceId: dataResourceId,
dataResourceTypeId: 'ELECTRONIC_TEXT'])
+ delegator.create('ElectronicText', [dataResourceId: dataResourceId,
textData: 'Test text for typed custom method check'])
+ // CONTENT_RENDER is shipped seed data
(framework/common/data/CommonTypeData.xml)
+ delegator.create('CustomMethod', [customMethodId: customMethodId,
customMethodTypeId: 'CONTENT_RENDER',
+ customMethodName: 'getDataResource'])
+ delegator.create('Content', [contentId: contentId, contentTypeId:
'TEST_CONTENT_TYPE',
+ dataResourceId: dataResourceId, customMethodId:
customMethodId])
+ GenericValue content = from('Content').where('contentId',
contentId).queryOne()
+ assert content
+
+ StringWriter out = new StringWriter()
+ Map<String, Object> templateContext = [userLogin: userLogin,
+ requestParameters:
[dataResourceId: dataResourceId]]
+ ContentWorker.renderContentAsText(dispatcher, content, out,
templateContext, Locale.US, 'text/plain', false, null)
+ // the invoked service's OUT parameters were merged into
templateContext
+ assert templateContext.resultData
+ assert out.toString().contains('Test text for typed custom method
check')
+ }
+
+ @Test
+ @Order(11)
+ void testRenderContentAsTextRejectsCustomMethodOfWrongType() {
+ String contentId = testParams.contentId ?: 'TEST_CNT_CM_WRONG'
+ String customMethodId = testParams.customMethodId ?:
'TEST_CM_WRONG_TYPE'
+ // TELECOM_GATEWAY is shipped seed data
(framework/common/data/CommonTypeData.xml), unrelated to
+ // content rendering - any type other than CONTENT_RENDER proves the
point
+ delegator.create('CustomMethod', [customMethodId: customMethodId,
customMethodTypeId: 'TELECOM_GATEWAY',
+ customMethodName: 'getDataResource'])
+ delegator.create('Content', [contentId: contentId, contentTypeId:
'TEST_CONTENT_TYPE',
+ dataResourceId: 'TEST_CONTENT_TEXT1', customMethodId:
customMethodId])
+ GenericValue content = from('Content').where('contentId',
contentId).queryOne()
+ assert content
+
+ StringWriter out = new StringWriter()
+ Map<String, Object> templateContext = [userLogin: userLogin,
requestParameters: [:]]
+ GeneralException exception = assertThrows(GeneralException) {
+ ContentWorker.renderContentAsText(dispatcher, content, out,
templateContext, Locale.US, 'text/plain', false, null)
+ }
+ assert exception.message.contains('not a content rendering method')
+ }
+
}
diff --git a/applications/product/servicedef/services.xml
b/applications/product/servicedef/services.xml
index 2e429a07c4..20e710dc88 100644
--- a/applications/product/servicedef/services.xml
+++ b/applications/product/servicedef/services.xml
@@ -366,6 +366,10 @@ under the License.
<auto-attributes include="nonpk" mode="IN" optional="true"/>
<auto-attributes entity-name="Content" include="nonpk" mode="IN"
optional="true"/>
<override name="fromDate" default-value="${date:nowTimestamp()}"/>
+ <!-- serviceName/customMethodId choose what code renderContentAsText
runs against the live request
+ parameters; neither is set through this generic mass-assignment
path (see ContentWorker.java) -->
+ <override name="serviceName" mode="OUT"/>
+ <override name="customMethodId" mode="OUT"/>
</service>
<service name="updateProductContent" default-entity-name="ProductContent"
engine="groovy"
location="component://product/src/main/groovy/org/apache/ofbiz/product/product/product/ProductContentServicesScript.groovy"
invoke="updateProductContent" auth="true">
@@ -373,6 +377,10 @@ under the License.
<auto-attributes include="pk" mode="IN" optional="false"/>
<auto-attributes include="nonpk" mode="IN" optional="true"/>
<auto-attributes entity-name="Content" include="nonpk" mode="IN"
optional="true"/>
+ <!-- serviceName/customMethodId choose what code renderContentAsText
runs against the live request
+ parameters; neither is set through this generic mass-assignment
path (see ContentWorker.java) -->
+ <override name="serviceName" mode="OUT"/>
+ <override name="customMethodId" mode="OUT"/>
</service>
<service name="removeProductContent" default-entity-name="ProductContent"
engine="entity-auto" invoke="delete" auth="true">
<description>Remove Content From Product</description>
diff --git a/applications/product/widget/catalog/ProductForms.xml
b/applications/product/widget/catalog/ProductForms.xml
index 22b906158a..40c1f9af7b 100644
--- a/applications/product/widget/catalog/ProductForms.xml
+++ b/applications/product/widget/catalog/ProductForms.xml
@@ -773,7 +773,6 @@ under the License.
<field name="description"
title="${uiLabelMap.ProductProductDescription}" map-name="content"><text
size="40"/></field>
<field name="fromDate" title="${uiLabelMap.CommonFromDate}" ></field>
<field name="thruDate" title="${uiLabelMap.CommonThruDate}"></field>
- <field name="serviceName" title="${uiLabelMap.ProductServiceName}"
map-name="content"><text size="40"/></field>
<field use-when="contentId == null" name="contentId"
title="${uiLabelMap.ProductContentId}"
tooltip="${uiLabelMap.ProductOptional}"><text maxlength="20"/></field>
<field use-when="contentId != null" name="contentId"
title="${uiLabelMap.ProductContentId}"
tooltip="${uiLabelMap.ProductNotModificationRecrationProductContentAssociation}"
map-name="productContentData" ><display/></field>
<field name="productId"><hidden/></field>
diff --git a/framework/common/data/CommonTypeData.xml
b/framework/common/data/CommonTypeData.xml
index 70c32c91b3..53c675eed6 100644
--- a/framework/common/data/CommonTypeData.xml
+++ b/framework/common/data/CommonTypeData.xml
@@ -152,4 +152,7 @@ under the License.
<TelecomMethodType telecomMethodTypeId="SMS" description="Short Messaging
Service Method"/>
<TelecomMethodType telecomMethodTypeId="WHATSAPP" description="WhatsApp
Messaging Service Method"/>
<CustomMethodType customMethodTypeId="TELECOM_GATEWAY"
description="Telecom Gateway Custom Method"/>
+
+ <!-- Content Rendering Custom Method -->
+ <CustomMethodType customMethodTypeId="CONTENT_RENDER" description="Content
Rendering Method"/>
</entity-engine-xml>
