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 4cc9848625 Fixed: updateLayout authorizing under CONTENT_CREATE then
updating unrelated records unchecked (#1745)
4cc9848625 is described below
commit 4cc9848625cfe4d22765167b6476573752f74c82
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Wed Aug 26 22:56:43 2026 +0530
Fixed: updateLayout authorizing under CONTENT_CREATE then updating
unrelated records unchecked (#1745)
The updateLayout event built a Content value from caller-supplied
parameters and checked permission for it under the CONTENT_CREATE
operation, then separately loaded the real Content identified by
parameters.contentId and the real DataResource identified by
parameters.dataResourceId and stored both directly, without checking
UPDATE permission on either record and without any check that the
DataResource actually belonged to the Content.
A user holding only view-level Content Manager permissions could use an
existing RESPONSE-purpose Content to pass the CONTENT_CREATE check, then
relink that Content to an arbitrary DataResource and convert it into a
template-bearing (FTL) OFBIZ_FILE resource - a path the normal
updateDataResource service already blocks for the same user, since it
requires CONTENTMGR_SUPER for template-bearing DataResources.
Route both updates through the existing updateContent/updateDataResource
services instead, so permission is checked (via
genericContentPermission/ genericDataResourcePermission, main-action
UPDATE) against the real target records, the same way every other update
path in this component already works.
Added a regression test that creates a low-privilege user with only
OFBTOOLS_VIEW/CONTENTMGR_VIEW and verifies it is denied both when
updating an existing Content and when converting an existing
DataResource into a template-bearing OFBIZ_FILE resource.
Thank you Krishna Uprit for your help in reviewing the changes.
---
.../content/minilang/layout/LayoutEvents.xml | 43 ++++----
.../permission/UpdateLayoutPermissionTests.groovy | 114 +++++++++++++++++++++
applications/content/testdef/ContentTests.xml | 4 +
3 files changed, 138 insertions(+), 23 deletions(-)
diff --git a/applications/content/minilang/layout/LayoutEvents.xml
b/applications/content/minilang/layout/LayoutEvents.xml
index 078218c7a7..692b7279e2 100644
--- a/applications/content/minilang/layout/LayoutEvents.xml
+++ b/applications/content/minilang/layout/LayoutEvents.xml
@@ -52,37 +52,34 @@ under the License.
<simple-method login-required="true" method-name="updateLayout"
short-description="Update Layout">
<log level="info" message="in updateLayout."/>
- <make-value entity-name="Content" value-field="currentContent"/>
- <set-pk-fields map="parameters" value-field="currentContent"/>
- <set-nonpk-fields map="parameters" value-field="currentContent"/>
- <set field="context.currentContent" from-field="currentContent"/>
+ <!-- Route the actual persistence through
updateContent/updateDataResource so permission is
+ checked (via
genericContentPermission/genericDataResourcePermission, main-action UPDATE)
+ against the real target Content and DataResource records
identified by parameters.contentId
+ and parameters.dataResourceId, instead of against an
attacker-constructed value under the
+ unrelated CONTENT_CREATE operation. This also ensures converting
the target DataResource
+ into a template-bearing (e.g. FTL) resource is gated behind
CONTENTMGR_SUPER, same as the
+ normal updateDataResource path. -->
+ <make-value entity-name="Content" value-field="content"/>
+ <set-pk-fields map="parameters" value-field="content"/>
+ <set-nonpk-fields map="parameters" value-field="content"/>
+ <log level="info" message="content: ${content}"/>
+ <map-to-map map="content" to-map="context"/>
<set field="context.contentPurposeTypeId"
from-field="parameters.contentPurposeTypeId"/>
- <string-to-list list="targetOperationList" string="CONTENT_CREATE"/>
- <set field="context.targetOperationList"
from-field="targetOperationList"/>
<session-to-field field="context.userLogin" session-name="userLogin"/>
- <call-service in-map-name="context"
service-name="checkContentPermission">
- <result-to-field result-name="permissionStatus"
field="permissionStatus"/>
- </call-service>
- <if-compare field="permissionStatus" operator="not-equals"
value="granted" type="String">
- <add-error>
- <fail-property resource="ContentUiLabels"
property="ContentPermissionNotGranted"/>
- </add-error>
- </if-compare>
+ <call-service in-map-name="context" service-name="updateContent"/>
<check-errors/>
- <set field="content.contentId" from-field="parameters.contentId"/>
- <find-by-primary-key entity-name="Content" value-field="content"
map="content"/>
- <set-nonpk-fields map="parameters" value-field="content"/>
- <log level="info" message="content: ${content}"/>
- <store-value value-field="content"/>
<field-to-request field="content.contentId" request-name="contentId"/>
- <set field="dataResource.dataResourceId"
from-field="parameters.dataResourceId"/>
- <find-by-primary-key entity-name="DataResource"
value-field="dataResource" map="dataResource"/>
+
+ <make-value entity-name="DataResource" value-field="dataResource"/>
+ <set-pk-fields map="parameters" value-field="dataResource"/>
<set-nonpk-fields map="parameters" value-field="dataResource"/>
<set field="dataResource.objectInfo"
from-field="parameters.drObjectInfo"/>
<log level="info" message="dataResource: ${dataResource}"/>
- <store-value value-field="dataResource"/>
- <field-to-request field="dataResource.dataResourceId"
request-name="drDataResourceId"/>
+ <map-to-map map="dataResource" to-map="context2"/>
+ <session-to-field field="context2.userLogin" session-name="userLogin"/>
+ <call-service in-map-name="context2"
service-name="updateDataResource"/>
<check-errors/>
+ <field-to-request field="dataResource.dataResourceId"
request-name="drDataResourceId"/>
</simple-method>
<simple-method login-required="true" method-name="createLayoutText"
short-description="Create Layout Text">
diff --git
a/applications/content/src/test/groovy/org/apache/ofbiz/content/permission/UpdateLayoutPermissionTests.groovy
b/applications/content/src/test/groovy/org/apache/ofbiz/content/permission/UpdateLayoutPermissionTests.groovy
new file mode 100644
index 0000000000..2f3aa882f5
--- /dev/null
+++
b/applications/content/src/test/groovy/org/apache/ofbiz/content/permission/UpdateLayoutPermissionTests.groovy
@@ -0,0 +1,114 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+
*******************************************************************************/
+package org.apache.ofbiz.content.permission
+
+import static org.junit.jupiter.api.Assertions.assertThrows
+
+import java.sql.Timestamp
+
+import org.apache.ofbiz.entity.GenericValue
+import org.apache.ofbiz.service.ServiceAuthException
+import org.apache.ofbiz.testtools.JunitJupiterTest
+import org.apache.ofbiz.testtools.JupiterTestHelper
+import org.junit.jupiter.api.Test
+
+/**
+ * Regression coverage for the updateLayout permission bypass: the event used
to authorize its
+ * attacker-supplied target Content under the unrelated CONTENT_CREATE
operation and then update
+ * the real Content and DataResource records directly, without checking UPDATE
permission on
+ * either one. It now delegates both updates to the
updateContent/updateDataResource services,
+ * which check permission (via
genericContentPermission/genericDataResourcePermission, main-action
+ * UPDATE) against the real target records. These tests exercise exactly that
permission boundary
+ * for a low-privilege user - one whose only grants are OFBTOOLS_VIEW and
CONTENTMGR_VIEW, with no
+ * CONTENTMGR admin permission and no role or ownership standing on the target
records - the same
+ * profile the reported exploit used.
+ */
+@JunitJupiterTest
+class UpdateLayoutPermissionTests implements JupiterTestHelper {
+
+ // Content/DataResource/SecurityGroup ids are "id" fields (VARCHAR(20));
keep these <= 20 chars
+ private static final String GROUP_ID = 'TEST_LAYOUT_VIEW'
+ private static final String USER_LOGIN_ID = 'testLayoutViewOnlyUser'
+ private static final String CONTENT_ID = 'TEST_LAYOUT_RESP_CT'
+ private static final String DATA_RESOURCE_ID = 'TEST_LAYOUT_TGT_DR'
+ private static final Timestamp GRANT_FROM_DATE =
Timestamp.valueOf('2020-01-01 00:00:00.0')
+
+ @Test
+ void testViewOnlyUserCannotUpdateContentOrConvertDataResourceToTemplate() {
+ GenericValue lowPrivUserLogin = createViewOnlyUserLogin()
+ GenericValue content = createTestContent()
+ GenericValue dataResource = createTestDataResource()
+
+ // relinking/renaming the real Content record must be denied without
UPDATE standing on it;
+ // a failed permission-service check surfaces as ServiceAuthException
out of runSync, not as
+ // a returned error-result map
+ assertThrows(ServiceAuthException) {
+ dispatcher.runSync('updateContent', [contentId: content.contentId,
+ contentName: 'Hijacked by view-only user', userLogin:
lowPrivUserLogin])
+ }
+
+ // converting the real DataResource into a template-bearing OFBIZ_FILE
resource must be
+ // denied without CONTENTMGR_SUPER, regardless of its current
(non-template) type
+ assertThrows(ServiceAuthException) {
+ dispatcher.runSync('updateDataResource', [dataResourceId:
dataResource.dataResourceId,
+ dataResourceTypeId: 'OFBIZ_FILE', dataTemplateTypeId:
'FTL',
+ objectInfo: 'runtime/logs/access_log.txt', userLogin:
lowPrivUserLogin])
+ }
+ }
+
+ private GenericValue createViewOnlyUserLogin() {
+ GenericValue existing = from('UserLogin').where(userLoginId:
USER_LOGIN_ID).queryOne()
+ if (existing) {
+ return existing
+ }
+
+ if (!from('SecurityGroup').where(groupId: GROUP_ID).queryOne()) {
+ delegator.create('SecurityGroup', [groupId: GROUP_ID,
+ description: 'View-only group for the updateLayout
permission regression test'])
+ delegator.create('SecurityGroupPermission', [groupId: GROUP_ID,
permissionId: 'OFBTOOLS_VIEW',
+ fromDate: GRANT_FROM_DATE])
+ delegator.create('SecurityGroupPermission', [groupId: GROUP_ID,
permissionId: 'CONTENTMGR_VIEW',
+ fromDate: GRANT_FROM_DATE])
+ }
+
+ GenericValue userLogin = delegator.create('UserLogin', [userLoginId:
USER_LOGIN_ID, enabled: 'Y'])
+ delegator.create('UserLoginSecurityGroup', [userLoginId:
USER_LOGIN_ID, groupId: GROUP_ID,
+ fromDate: GRANT_FROM_DATE])
+ return userLogin
+ }
+
+ private GenericValue createTestContent() {
+ GenericValue existing = from('Content').where(contentId:
CONTENT_ID).queryOne()
+ if (existing) {
+ return existing
+ }
+ return delegator.create('Content', [contentId: CONTENT_ID,
contentTypeId: 'DOCUMENT',
+ contentName: 'Test Layout Response Content'])
+ }
+
+ private GenericValue createTestDataResource() {
+ GenericValue existing = from('DataResource').where(dataResourceId:
DATA_RESOURCE_ID).queryOne()
+ if (existing) {
+ return existing
+ }
+ return delegator.create('DataResource', [dataResourceId:
DATA_RESOURCE_ID,
+ dataResourceTypeId: 'ELECTRONIC_TEXT', mimeTypeId:
'text/plain'])
+ }
+
+}
diff --git a/applications/content/testdef/ContentTests.xml
b/applications/content/testdef/ContentTests.xml
index 1b02492356..3a457a0b44 100644
--- a/applications/content/testdef/ContentTests.xml
+++ b/applications/content/testdef/ContentTests.xml
@@ -28,5 +28,9 @@
<test-case case-name="content-tests">
<jupiter-test-suite
class-name="org.apache.ofbiz.content.content.ContentTests"/>
</test-case>
+
+ <test-case case-name="update-layout-permission-tests">
+ <jupiter-test-suite
class-name="org.apache.ofbiz.content.permission.UpdateLayoutPermissionTests"/>
+ </test-case>
</test-suite>
