Cherry-picks "WW-5417 update OgnlRuntime & ObjectPropertyAccessor to do access check for set field value"

.github/workflows/maven.yml

@@ -49,28 +49,16 @@ jobs:
           server-password: MAVEN_PASSWORD
       - name: Build with Maven on Java ${{ matrix.java }}
         if: matrix.java != '17'
-        run: mvn -B -V test --no-transfer-progress
+        run: ./mvnw -B -V test --no-transfer-progress
       - name: Code coverage
         if: matrix.java == '17'
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
-        run: mvn -B -V -Pcoverage verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar --no-transfer-progress
+        run: ./mvnw -B -V -Pcoverage verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar --no-transfer-progress
       - name: Deploy SNAPSHOT
         if: matrix.java == '8' && github.ref == 'refs/heads/master'
         env:
           MAVEN_USERNAME: ${{ secrets.OSSRH_USERNAME }}
           MAVEN_PASSWORD: ${{ secrets.OSSRH_TOKEN }}
-        run: mvn -B -V -DskipTests=true deploy --no-transfer-progress
-      - name: Apply automerge label
-        if: matrix.java == '17' && github.actor == 'dependabot[bot]'
-        run: gh pr edit "$PR_URL" --add-label "automerge"
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          PR_URL: ${{ github.event.pull_request.html_url }}
-      - name: Automerge
-        if: matrix.java == '17' && github.actor == 'dependabot[bot]'
-        run: gh pr merge -m "$PR_URL"
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          PR_URL: ${{ github.event.pull_request.html_url }}
+        run: ./mvnw -B -V -DskipTests=true deploy --no-transfer-progress

.mvn/wrapper/maven-wrapper.properties

@@ -1 +1 @@
-distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.5.2/apache-maven-3.5.2-bin.zip
+distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.6.3/apache-maven-3.6.3-bin.zip

src/main/java/ognl/ObjectPropertyAccessor.java

@@ -71,12 +71,12 @@ public Object setPossibleProperty(OgnlContext context, Object target, String nam
         Object result = null;
         try {
             if (!OgnlRuntime.setMethodValue(context, target, name, value, true)) {
-                result = OgnlRuntime.setFieldValue(context, target, name, value) ? null : OgnlRuntime.NotFound;
+                result = OgnlRuntime.setFieldValue(context, target, name, value, true) ? null : OgnlRuntime.NotFound;
             }
 
             if (result == OgnlRuntime.NotFound) {
                 Method m = OgnlRuntime.getWriteMethod(target.getClass(), name);
-                if (m != null) {
+                if (m != null && context.getMemberAccess().isAccessible(context, target, m, name)) {
                     result = m.invoke(target, value);
                 }
             }

src/main/java/ognl/OgnlRuntime.java

@@ -1648,7 +1648,7 @@ public static Object callConstructor(OgnlContext context, String className, Obje
                     throw new NoSuchMethodException();
                 }
             }
-            if (!context.getMemberAccess().isAccessible(context, target, ctor, null)) {
+            if (!isAccessible(context, target, ctor, null)) {
                 throw new IllegalAccessException(
                         "access denied to " + target.getName() + "()");
             }
@@ -1702,7 +1702,7 @@ public static Object getMethodValue(OgnlContext context, Object target, String p
             m = getReadMethod((target == null) ? null : target.getClass(), propertyName, null);
 
         if (checkAccessAndExistence) {
-            if ((m == null) || !context.getMemberAccess().isAccessible(context, target, m, propertyName)) {
+            if ((m == null) || !isAccessible(context, target, m, propertyName)) {
                 result = NotFound;
             }
         }
@@ -1740,7 +1740,7 @@ public static boolean setMethodValue(OgnlContext context, Object target, String
         Method m = getSetMethod(context, (target == null) ? null : target.getClass(), propertyName);
 
         if (checkAccessAndExistence) {
-            if ((m == null) || !context.getMemberAccess().isAccessible(context, target, m, propertyName)) {
+            if ((m == null) || !isAccessible(context, target, m, propertyName)) {
                 result = false;
             }
         }
@@ -1863,7 +1863,7 @@ public static Object getFieldValue(OgnlContext context, Object target, String pr
         final Field f = getField((target == null) ? null : target.getClass(), propertyName);
 
         if (checkAccessAndExistence) {
-            if ((f == null) || !context.getMemberAccess().isAccessible(context, target, f, propertyName)) {
+            if ((f == null) || !isAccessible(context, target, f, propertyName)) {
                 result = NotFound;
             }
         }
@@ -1892,16 +1892,27 @@ public static Object getFieldValue(OgnlContext context, Object target, String pr
         return result;
     }
 
-    public static boolean setFieldValue(OgnlContext context, Object target, String propertyName, Object value)
-            throws OgnlException {
+
+    /**
+     * Don't use this method as it doesn't check member access rights via {@link MemberAccess} interface
+     */
+    @Deprecated
+    public static boolean setFieldValue(OgnlContext context, Object target, String propertyName, Object value) throws OgnlException {
+        return setFieldValue(context, target, propertyName, value, false);
+    }
+
+    public static boolean setFieldValue(OgnlContext context, Object target, String propertyName, Object value,
+                                        boolean checkAccessAndExistence)
+            throws OgnlException
+    {
         boolean result = false;
 
         try {
             final Field f = getField((target == null) ? null : target.getClass(), propertyName);
 
             if (f != null) {
                 final int fModifiers = f.getModifiers();
-                if (!Modifier.isStatic(fModifiers) && !Modifier.isFinal(fModifiers)) {
+                if (!Modifier.isStatic(fModifiers) && !Modifier.isFinal(fModifiers) && (!checkAccessAndExistence || isAccessible(context, target, f, propertyName))) {
                     final Object state = context.getMemberAccess().setup(context, target, f, propertyName);
                     try {
                         if (isTypeCompatible(value, f.getType())
@@ -1925,7 +1936,7 @@ public static boolean isFieldAccessible(OgnlContext context, Object target, Clas
     }
 
     public static boolean isFieldAccessible(OgnlContext context, Object target, Field field, String propertyName) {
-        return context.getMemberAccess().isAccessible(context, target, field, propertyName);
+        return isAccessible(context, target, field, propertyName);
     }
 
     public static boolean hasField(OgnlContext context, Object target, Class<?> inClass, String propertyName) {
@@ -1977,7 +1988,7 @@ public static Object getStaticField(OgnlContext context, String className, Strin
             }
 
             Object result;
-            if (context.getMemberAccess().isAccessible(context, null, f, null)) {
+            if (isAccessible(context, null, f, null)) {
                 final Object state = context.getMemberAccess().setup(context, null, f, null);
                 try {
                     result = f.get(null);
@@ -2134,7 +2145,7 @@ private static Method _getGetMethod(Class<?> targetClass, String propertyName) {
     }
 
     public static boolean isMethodAccessible(OgnlContext context, Object target, Method method, String propertyName) {
-        return (method != null) && context.getMemberAccess().isAccessible(context, target, method, propertyName);
+        return (method != null) && isAccessible(context, target, method, propertyName);
     }
 
     public static boolean hasGetMethod(OgnlContext context, Object target, Class<?> targetClass, String propertyName) {
@@ -3032,4 +3043,17 @@ public static boolean getUseFirstMatchGetSetLookupValue() {
         return _useFirstMatchGetSetLookup;
     }
 
+    /**
+     * Returns true if the given member is accessible or can be made accessible
+     * by this object.
+     *
+     * @param context the current execution context.
+     * @param target the Object to test accessibility for.
+     * @param member the Member to test accessibility for.
+     * @param propertyName the property to test accessibility for.
+     * @return true if the target/member/propertyName is accessible in the context, false otherwise.
+     */
+    private static boolean isAccessible(OgnlContext context, Object target, Member member, String propertyName) {
+        return context.getMemberAccess().isAccessible(context, target, member, propertyName);
+    }
 }

src/test/java/ognl/ExcludedObjectMemberAccess.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2020 OGNL Contributors
+ *
+ * Licensed 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 ognl;
+
+import java.lang.reflect.Member;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * This class provides simple functionality for mark / unmark an object as inaccessible
+ */
+public class ExcludedObjectMemberAccess extends DefaultMemberAccess {
+    private final List<Object> excludedObjects = new ArrayList<>(); // Any field or method in this list will be inaccessible
+
+    public ExcludedObjectMemberAccess(boolean allowAllAccess) {
+        super(allowAllAccess);
+    }
+
+    public ExcludedObjectMemberAccess(boolean allowPrivateAccess, boolean allowProtectedAccess, boolean allowPackageProtectedAccess) {
+        super(allowPrivateAccess, allowProtectedAccess, allowPackageProtectedAccess);
+    }
+
+    public boolean isAccessible(OgnlContext context, Object target, Member member, String propertyName) {
+        if (excludedObjects.contains(member)) {
+            return false;
+        }
+
+        return super.isAccessible(context, target, member, propertyName);
+    }
+
+    public void exclude(Object obj) {
+        excludedObjects.add(obj);
+    }
+
+    public void removeExclusion(Object obj) {
+        excludedObjects.remove(obj);
+    }
+}

src/test/java/ognl/TestObjectPropertyAccessor.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2020 OGNL Contributors
+ *
+ * Licensed 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 ognl;
+
+import junit.framework.TestCase;
+
+import java.beans.IntrospectionException;
+import java.lang.reflect.Method;
+import java.util.Map;
+
+/**
+ * Tests various methods / functionality of {@link ObjectPropertyAccessor}.
+ */
+public class TestObjectPropertyAccessor extends TestCase {
+    private Map context;
+    private ObjectPropertyAccessor propertyAccessor;
+
+    public void setUp() throws Exception {
+        super.setUp();
+        context = Ognl.createDefaultContext(null, new ExcludedObjectMemberAccess(false));
+        propertyAccessor = new ObjectPropertyAccessor();
+    }
+
+    /**
+     * Public class for "setPossibleProperty" method tests.
+     */
+    public static class SimplePublicClass {
+        private String gender = "male";
+        public String email = "test@test.com";
+        private String name = "name";
+        private String age = "18";
+
+        public void setGender(String gender) {
+            this.gender = gender;
+        }
+
+        private void setEmail(String email) {
+            this.email = email;
+        }
+
+        private void setName(String email) {
+            this.email = email;
+        }
+
+        public void setname(String name) {
+            this.name = name;
+        }
+
+        private void setAge(String age) {
+            this.age = age;
+        }
+
+        public void setage(String age) {
+            this.age = age;
+        }
+    }
+
+    public void testSetPossibleProperty() throws OgnlException, IntrospectionException {
+        OgnlContext context = (OgnlContext) this.context;
+        SimplePublicClass simplePublic = new SimplePublicClass();
+
+        // 1. when set method is accessible and set method
+        assertNotSame(OgnlRuntime.NotFound, propertyAccessor.setPossibleProperty(context, simplePublic, "gender", "female"));
+        assertEquals("female", simplePublic.gender);
+
+        // 2. when set method is NOT accessible and fallback to set field (field is accessible)
+        assertNotSame(OgnlRuntime.NotFound, propertyAccessor.setPossibleProperty(context, simplePublic, "email", "admin@admin.com"));
+        assertEquals("admin@admin.com", simplePublic.email);
+
+        // 3. when set method is NOT accessible, field is NOT accessible, fallback to write method (write method is accessible)
+        assertEquals("setName", OgnlRuntime.getSetMethod(context, SimplePublicClass.class, "name").getName());
+        assertEquals("setname", OgnlRuntime.getWriteMethod(SimplePublicClass.class, "name", null).getName());
+        assertNotSame(OgnlRuntime.NotFound, propertyAccessor.setPossibleProperty(context, simplePublic, "name", "new name"));
+        assertEquals("new name", simplePublic.name);
+
+        // 4. when set method is NOT accessible, field is NOT accessible, fallback to write method (write method is NOT accessible)
+        Method ageWriteMethod = OgnlRuntime.getWriteMethod(SimplePublicClass.class, "age", null);
+        ((ExcludedObjectMemberAccess) context.getMemberAccess()).exclude(ageWriteMethod);
+
+        assertEquals("setage", ageWriteMethod.getName());
+        assertFalse(context.getMemberAccess().isAccessible(context, simplePublic, ageWriteMethod, "age"));
+        assertEquals("setAge", OgnlRuntime.getSetMethod(context, SimplePublicClass.class, "age").getName());
+        assertEquals(OgnlRuntime.NotFound, propertyAccessor.setPossibleProperty(context, simplePublic, "age", "99"));
+        assertEquals("18", simplePublic.age);
+    }
+}