From 64bd12b26da0916b088910c21e1852ef1f2ccaeb Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Wed, 30 Jan 2019 18:31:51 -0500 Subject: [PATCH] Fix for access issue for 2.6 discovered in WW-5004 (Minor update to previous commit): - Restored ability to access public static fields (true by default). - Introduced a boolean configuration flag (allowStaticFieldAccess). - Replaced one remaining Boolean.parseBoolean() conversion in OgnlUtil use BooleanUtils.toBoolean(). - Enhanced unit tests to confirm proper operation of the fix. - Replicating L. Lenart's change in PR#317: - Removed injection parameter for setAllowStaticMethodAccess in OgnlValueStackFactory. - Replaced with lazy retrieval of allowStaticMethodAccess from container. - Used same pattern for the new allowStaticFieldAccess flag. - Added retrieval methods for both flags from the container. - Optimized calling sequence of isAccessible() based on feedback from previous commit. - Made a couple of getters and the protected checkXXX methods final (avoid descendant interference). --- .../xwork2/ognl/SecurityMemberAccess.java | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 9174185a4..fd36972a5 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -63,11 +63,11 @@ public class SecurityMemberAccess implements MemberAccess { this.allowStaticFieldAccess = allowStaticFieldAccess; } - public boolean getAllowStaticMethodAccess() { + public final boolean getAllowStaticMethodAccess() { return allowStaticMethodAccess; } - public boolean getAllowStaticFieldAccess() { + public final boolean getAllowStaticFieldAccess() { return allowStaticFieldAccess; } @@ -110,13 +110,18 @@ public class SecurityMemberAccess implements MemberAccess { return true; } - if (!checkStaticMemberAccess(member)) { + final int memberModifiers = member.getModifiers(); + if (!checkStaticMemberAccess(member, memberModifiers)) { LOG.warn("Access to static [{}] is blocked!", member); return false; } + if (!checkPublicMemberAccess(memberModifiers)) { + LOG.trace("Access to non-public [{}] is blocked!", member); + return false; + } + final Class memberClass = member.getDeclaringClass(); - final int memberModifiers = member.getModifiers(); if (isClassExcluded(memberClass)) { LOG.warn("Declaring class of member type [{}] is excluded!", member); @@ -142,37 +147,51 @@ public class SecurityMemberAccess implements MemberAccess { return false; } - return Modifier.isPublic(memberModifiers) && isAcceptableProperty(propertyName); + return isAcceptableProperty(propertyName); } /** - * Check access for static members + * Check access for static members (via modifiers) + * + * Static non-field access result is allowStaticMethodAccess. + * Static field access result is allowStaticFieldAccess. * - * Static non-field access result is a logical and of allowStaticMethodAccess and public. - * Static field access result is a logical and of allowStaticFieldAccess and public. * Note: For non-static members, the result is always true. * * @param member + * @param memberModifiers (minor optimization) * * @return */ - protected boolean checkStaticMemberAccess(Member member) { - final int modifiers = member.getModifiers(); - if (Modifier.isStatic(modifiers)) { + protected final boolean checkStaticMemberAccess(Member member, int memberModifiers) { + if (Modifier.isStatic(memberModifiers)) { if (member instanceof Field) { - return allowStaticFieldAccess && Modifier.isPublic(modifiers); + return allowStaticFieldAccess; } else { if (allowStaticMethodAccess) { LOG.debug("Support for accessing static methods [member: {}] is deprecated!", member); } - return allowStaticMethodAccess && Modifier.isPublic(modifiers); + return allowStaticMethodAccess; } } else { return true; } } - protected boolean checkEnumAccess(Object target, Member member) { + /** + * Check access for public members (via modifiers) + * + * Returns true if-and-only-if the member is public. + * + * @param memberModifiers + * + * @return + */ + protected final boolean checkPublicMemberAccess(int memberModifiers) { + return Modifier.isPublic(memberModifiers); + } + + protected final boolean checkEnumAccess(Object target, Member member) { if (target instanceof Class) { final Class clazz = (Class) target; if (Enum.class.isAssignableFrom(clazz) && member.getName().equals("values")) {