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-5349 Remove Struts core dependency on OGNL VarRefs #763

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

kusalk
Copy link
Member

@kusalk kusalk commented Oct 9, 2023

WW-5349

For applications that don't use components such as Action and Set, or templates which leverage OGNL variable references, #attr.templateDir and #attr.theme are the only 2 dependencies on this OGNL capability.

By removing this functionality, applications can block ognl.ASTVarRef using struts.ognl.excludedNodeTypes for strengthened security. This is a commonly used gadget by attackers.

@kusalk kusalk requested a review from lukaszlenart October 9, 2023 04:57
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 9, 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 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@lukaszlenart
Copy link
Member

Wouldn't make sense to exclude ognl.ASTVarRef by default?

@kusalk
Copy link
Member Author

kusalk commented Oct 9, 2023

I think many applications make use of the VarRef functionality (including any application that uses Action or Set Struts components) so no it wouldn't make sense to block by default.

But by removing this dependency in UIBean, for applications that don't use this functionality, they can choose to block it completely.

I've got a list of other nodes that we probably can block by default (which I'm still compiling) - I think we can introduce that in 7.0.

@lukaszlenart
Copy link
Member

I've got a list of other nodes that we probably can block by default (which I'm still compiling) - I think we can introduce that in 7.0.

Nice, could you register a ticket to not forget about that?

is it still a draft?

@kusalk
Copy link
Member Author

kusalk commented Oct 12, 2023

Yeah still experimenting a bit. But essentially we should castrate OGNL as much as possible without hindering popular Struts features.

Here is the card for 7.0: WW-5353

@kusalk kusalk marked this pull request as ready for review October 12, 2023 08:26
@kusalk kusalk merged commit 913f6bf into master Oct 13, 2023
8 checks passed
@kusalk kusalk deleted the WW-5349-astvarref branch October 13, 2023 00:39
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