Repository navigation
Restrict access to property paths on Class references #28261
Description
No description provided.
Reacted by Andrei Damian-Fekete, Dominik Mengelt, Artem Smotrakov, lme-nca, Rick Ossendrijver, Philipp Sieber, ChangChe, Leo, Dan-Mikkel Hübenette, Marcel Stör and 5 more
Activity
artem-smotrakov commentedon Mar 31, 2022
Hi @rstoyanchev Does this address the recent vulnerability in Spring Framework?
Is Spring Framework 3.x vulnerable? I see the code is quite similar.
dicer commentedon Mar 31, 2022
@artem-smotrakov See https://spring.io/blog/2022/03/31/spring-framework-rce-early-announcement
artem-smotrakov commentedon Mar 31, 2022
Hi @dicer My question is exactly about this announcement :) I am looking for a commit that fixe this issue.
jhoeller commentedon Mar 31, 2022
@artem-smotrakov please note that the vulnerability only materializes on JDK 9+, and Spring Framework 3.x predated JDK 9+ by several years and did not even have full JDK 8 support yet. Running such old versions of Spring on a newer JDK version is technically possible for certain scenarios, but anyone doing so is on their own in terms of support.
Also, Spring Framework 3.2.18 (the latest release before 3.x EOL) is on a December 2016 security patch level. Continued usage of that version misses out on six years of vulnerability patches and defensiveness measures that we applied to 4.x and 5.x in the meantime. A patched version for the present CachedIntrospectionResults change would cover this particular attack vector on the unsupported JDK 9+ but might still keep you exposed to other vulnerabilities even in supported areas of the framework.
For older Spring versions, we strategically suggest an upgrade to 5.3.x, or tactically a workaround as suggested in our blog post: https://spring.io/blog/2022/03/31/spring-framework-rce-early-announcement
artem-smotrakov commentedon Mar 31, 2022
Hi @jhoeller All valid points, absolutely agree that it would be better to move to 5.3.x
If I understand correctly, this commit fixes the CVE-2022-22965. I'd appreciate if someone for the Spring team confirmed that. Thanks!
002546b
skydreamerr commentedon Apr 1, 2022
@bclozel we started to get exceptions after the upgrade.
The property has type
ClassLoader:https://github.com/eclipse/gemini.blueprint/blob/d671a74fe8aa631e8f006aad5e43d3b3f1be6359/core/src/main/java/org/eclipse/gemini/blueprint/service/importer/support/AbstractOsgiServiceImportFactoryBean.java#L215
marcelstoer commentedon Apr 1, 2022
I fully agree. I find it irritating that the issues here have no reference to the actual commit (or the other way around). That way you have to dig through the commit history and hope the commit message is the same as or similar enough to the issue title.
I did as you did and arrived at the same commit 👍
bclozel commentedon Apr 1, 2022
@skydreamerr please create a new issue for this.
jhoeller commentedon Apr 1, 2022
@skydreamerr Why is that "beanClassLoader" property being populated through declarative configuration?
setBeanClassLoaderis a programmatic callback that the container itself calls viaBeanClassLoaderAware, it's not meant to be populated explicitly. Please double-check the property configuration for that bean; maybe that explicit setting can simply be removed so that the container can perform its standard callback?rstoyanchev commentedon Apr 1, 2022
@marcelstoer normally commits are linked to issues. This was not a normal case.
marcelstoer commentedon Apr 1, 2022
I understand and appreciate this. Maybe just edit this issue (+ #28262) and add something like "Fixed by 002546b" to the description?
copernico commentedon Apr 1, 2022
Here you find the commits for both affected release series: https://github.com/SAP/project-kb/blob/vulnerability-data/statements/CVE-2022-22965/statement.yaml
(but I am not from the Spring team, so I am not providing the confirmation you were looking for :-| )
kennymacleod commentedon Apr 1, 2022
BeanClassLoaderAwareis being used by the Felix OSGi framework, so I can see they may have a legitimate use forBeanClassLoaderAware. In this particular case, the fix for the CVE seems to be breaking pre-existing and valid functionality.bclozel commentedon Apr 1, 2022
@kennymacleod please follow up on the dedicated issue here #28269, or create a new one if you believe your case is different.
What Juergen is merely pointing out here, is that the core container will honor this contract and that property configuration is not needed for this property.
jhoeller commentedon Apr 1, 2022
@kennymacleod I'm sure it's a valid use case for
BeanClassLoaderAwareitself but that container callback is meant to only be called by the container itself, not specified via declarative user configuration. That's the part I'd like to understand, there is either double configuration here - or accidental overriding since the container will perform its standardsetBeanClassLoaderAwarecall even when user configuration was passed into the same property before. Let's find out whether this is accidental for that specific OSGi setup scenario and whether it even had the intended effect before.That said, as per my recent comment on #28269, we are aware that this is technically a regression for isolated declarative configuration cases, as a side effect of a defensive security measure. If there are common scenarios where this regression remains unresolvable, we might specifically enable certain ClassLoader configuration scenarios again in a follow-up Spring Framework release.
Enerccio commentedon Apr 1, 2022
Are you sure that only Java 9+ is affected. By current attack vector, sure, but couldn't there be another one? What I mean is could there be another Class.property that can lead into this attack.
jhoeller commentedon Apr 1, 2022
To the best of our understanding, from a Spring perspective, this is indeed a Java 9+ only attack vector. It's essentially revisiting an old Class property attack vector that we closed for Java 6-8 many years ago, not having had any follow-ups in the meantime, now reopened at this late point for the unnoticed Java 9+ introduction of
Class.getModule().getClassLoader().That said, our new defensiveness checks in Spring's property descriptors cover any access to ClassLoader properties, on any JDK versions. We do recommend an upgrade to the latest framework version for defensiveness against unknown attack vectors, even on JDK 8, it is just not a necessary step to take for the present CVE with its Java 9+ attack vector.
Last but not least, since the present issue is about access to public facilities on Tomcat's ClassLoader implementation, we also strongly recommend an upgrade to the just-released Tomcat patches, in particular for attack vectors outside of Spring.
bclozel commentedon Apr 4, 2022
@grubeninspekteur as you can imagine, this has been a stressful week for the Spring community. Discussing publicly the effectiveness of the fix and potential security issues is not the responsible way to share your concerns.
Please use the dedicated channels for that: https://spring.io/security-policy
grubeninspekteur commentedon Apr 4, 2022
@bclozel Of course, the workload caused by the irresponsible disclosure of the vulnerability last week must have been immense and as Spring users we are grateful for your quick actions. I understand your policy of not discussing these matters publicly and have removed my comment.
JKAK47 commentedon Oct 11, 2022