-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add unsafe-deserialization support for Jabsorb #6419
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
Add unsafe-deserialization support for Jabsorb #6419
Conversation
This is partly extracted from github#5954
predicate looksLikeResolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNode) { | ||
exists(MethodAccess ma, Method m, Expr arg | m = ma.getMethod() and arg = ma.getAnArgument() | | ||
m.getReturnType() instanceof JacksonTypeDescriptorType and | ||
m.getName().toLowerCase().regexpMatch("(?i).*(resolve|load|class|type).*") and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m.getName().toLowerCase().regexpMatch("(?i).*(resolve|load|class|type).*") and | |
m.getName().regexpMatch("(?i).*(resolve|load|class|type).*") and |
@@ -77,14 +77,6 @@ class SetPolymorphicTypeValidatorSource extends DataFlow::ExprNode { | |||
} | |||
} | |||
|
|||
/** Holds if `fromNode` to `toNode` is a dataflow step that resolves a class. */ | |||
predicate resolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNode) { | |||
exists(ReflectiveClassIdentifierMethodAccess ma | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this removal make any imports superfluous in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, removed one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments, otherwise LGTM.
- Remove unneeded import - Remove unnecessary `toLowerCase` call
@aschackmull done |
or | ||
ma.getMethod() instanceof JabsorbUnmarshallMethod | ||
) and | ||
// Note `JacksonTypeDescriptorType` includes plain old `java.lang.Class` | ||
arg.getType() instanceof JacksonTypeDescriptorType and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should JacksonTypeDescriptorType
now be refactored as well given that the predicates using it are not exclusively used for Jackson anymore?
This is partly extracted from #5954