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

Deprecate SerializationUtils#deserialize #28075

Conversation

ledoyen
Copy link
Contributor

@ledoyen ledoyen commented Feb 18, 2022

As based on Java's serialization mechanism, it can be the source of Remote Code Execution vulnerabilities.

Today this utility is part of the public API and can be naively used to convert from object to text and vice versa.
However a naive use can lead to RCE vulnerability if user-input data (like files, cookies, etc.) is transfered using this utility.

I think it should be nice to at least warn the user about the use of this tool (with @Deprecated) and later on remove it totally from the public API as this sole use in Spring code is to clone exceptions in org.springframework.cache.jcache.interceptor.CacheResultInterceptor.

I am not sure on how it can (or should) be handled.
Let me know if you need me to adapt the code of this PR.

As based on Java's serialization mechanism, it can be the source of
Remote Code Execution vulnerabilities.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 18, 2022
@ttddyy
Copy link
Contributor

ttddyy commented Feb 18, 2022

I recently fixed a code using SerializationUtils#deserialize because it was flagged by a penetration test.
So, it would be nice to have such a notion, then users would be aware of the implication of using the method.

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Feb 19, 2022
@sbrannen sbrannen added this to the Triage Queue milestone Feb 19, 2022
@rstoyanchev rstoyanchev modified the milestones: Triage Queue, 6.0.0-M4 Mar 29, 2022
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 29, 2022
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Mar 29, 2022
@sbrannen sbrannen closed this in 7f7fb58 Mar 29, 2022
@sbrannen
Copy link
Member

This has been merged into main in 7f7fb58 and polished in c8d0146.

The "warning" without official deprecation has also been backported to 5.3.x (see #28246).

Thanks

@Tomator01
Copy link

when report this cve?

@ledoyen
Copy link
Contributor Author

ledoyen commented Mar 30, 2022

@Tomator01 This is not a CVE per se.

Using this tool to handle user input data can lead to a CVE.
However using it internally as CacheResultInterceptor was, will not result in a CVE.

@sbrannen
Copy link
Member

This is not a CVE in the core Spring Framework.

The purpose of this change is to inform anyone who had previously been using SerializationUtils#deserialize that it is dangerous to deserialize objects from untrusted sources.

The core Spring Framework does not use SerializationUtils to deserialize objects from untrusted sources.

If you believe you have discovered a security issue, please report it responsibly with the dedicated page: https://spring.io/security-policy

And please refrain from posting any additional comments to this commit.

Thank you

@spring-projects spring-projects locked as resolved and limited conversation to collaborators Mar 30, 2022
@ledoyen ledoyen deleted the fix/deprecate_rce_vulnerable_deserialization branch March 30, 2022 08:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants