Restrict allowed classes during deserialization of signature files #253
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Because signature files are created using Java Serialization, adds a new `SignatureObjectInputStream` which restricts the classes which are allowed to be loaded when reading signature files to increase security.
Marcono1234
commented
Jul 6, 2023
animal-sniffer/src/main/java/org/codehaus/mojo/animal_sniffer/ClassFileVisitor.java
Show resolved
Hide resolved
animal-sniffer/src/main/java/org/codehaus/mojo/animal_sniffer/Clazz.java
Show resolved
Hide resolved
animal-sniffer/src/main/java/org/codehaus/mojo/animal_sniffer/Clazz.java
Show resolved
Hide resolved
slachiewicz
approved these changes
Dec 26, 2023
slawekjaranowski
approved these changes
Dec 26, 2023
…/deserialization-restrictions
|
Thanks for your reviews! I just noticed a small typo in the tests and have pushed a commit for that and merged the changes from |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because signature files are created using Java Serialization, adds a new
SignatureObjectInputStreamwhich restricts the classes which are allowed to be loaded when reading signature files to increase security.I hope these changes are fine security-wise, but please let me know if I overlooked something or failed to consider something.
I have tested these changes with the following signatures to make sure that they can still be loaded successfully:
org.codehaus.mojo.signature:java12:1.0org.codehaus.mojo.signature:java18:1.0net.sf.androidscents.signature:android-api-level-1:1.0_r2net.sf.androidscents.signature:android-api-level-32:12_r1com.toasttab.android:gummy-bears-api-19:0.0.2com.toasttab.android:gummy-bears-api-33:0.5.1But feel free to perform additional tests to be safe
Relates to #252
But I don't think this resolves that issue fully or renders it obsolete. A different file format which is inherently safer would still be better than having to implement additional security measures on top of Java Serialization, as done in this pull request.