Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WW-5339 Add option to block custom OGNL maps #806

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Conversation

kusalk
Copy link
Member

@kusalk kusalk commented Dec 5, 2023

WW-5339

Having consulted with a security researcher - there is still some risk here if there exists a custom map implementation whose java.util.Map methods pose some risk. This is because ognl.MapPropertyAccessor#getProperty does not consult the MemberAccess policy for the inherent java.util.Map methods.

Given Struts does not rely on custom OGNL Map implementations, we should add an option to block this capability.

if (resolver == null) {
resolver = container.getInstance(CompoundRootAccessor.class);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't work :(

@kusalk kusalk force-pushed the WW-5339-astmap-block branch from 1a13187 to 002e598 Compare December 5, 2023 02:18
Copy link

sonarqubecloud bot commented Dec 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

84.4% 84.4% Coverage
1.9% 1.9% Duplication

Base automatically changed from WW-5364-populate-allowlist to master December 5, 2023 05:57
@kusalk kusalk marked this pull request as ready for review December 5, 2023 05:57
@kusalk kusalk requested a review from lukaszlenart December 5, 2023 06:02
Copy link
Member

@lukaszlenart lukaszlenart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, could you document another security option ? Thanks!

@kusalk
Copy link
Member Author

kusalk commented Dec 5, 2023

@lukaszlenart
Done :)
apache/struts-site#215

@kusalk kusalk merged commit 6fcb501 into master Dec 5, 2023
10 checks passed
@kusalk kusalk deleted the WW-5339-astmap-block branch December 5, 2023 06:36
@lukaszlenart
Copy link
Member

Just tested the Showcase app and it doesn't look good ;-)

[WARN ] ognl.SecurityMemberAccess (SecurityMemberAccess.java:205) - Declaring class [interface java.util.Map$Entry] of member type [public abstract java.lang.Object java.util.Map$Entry.getKey()] is not allowlisted!
[WARN ] ognl.SecurityMemberAccess (SecurityMemberAccess.java:163) - Access to non-public [final java.lang.Object java.util.HashMap$Node.key] is blocked!
[WARN ] ognl.SecurityMemberAccess (SecurityMemberAccess.java:163) - Access to non-public [protected java.lang.String org.apache.struts2.components.UIBean.key] is blocked!
[WARN ] ognl.SecurityMemberAccess (SecurityMemberAccess.java:205) - Declaring class [interface java.util.Map$Entry] of member type [public abstract java.lang.Object java.util.Map$Entry.getValue()] is not allowlisted!
[WARN ] ognl.SecurityMemberAccess (SecurityMemberAccess.java:163) - Access to non-public [java.lang.Object java.util.HashMap$Node.value] is blocked!
[WARN ] ognl.SecurityMemberAccess (SecurityMemberAccess.java:163) - Access to non-public [protected java.lang.String org.apache.struts2.components.UIBean.value] is blocked!
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants