-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Java: Add support for FastJson in unsafe deserialization. #4427
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
Conversation
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.
Could you add some test cases please.
*/ | ||
class FastJson extends RefType { | ||
FastJson() { | ||
this.hasQualifiedName("com.alibaba.fastjson", "JSON") or |
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.
I believe JSONObject
is a subclass of JSON
so there should be no need for the or
. There are also several other subclasses of JSON
. See: https://github.com/alibaba/fastjson/tree/master/src/main/java/com/alibaba/fastjson
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.
I've removed JSONObject
. It never really made sense to include, since the relevant methods are static.
*/ | ||
class FastJsonParseMethod extends Method { | ||
FastJsonParseMethod() { | ||
this.getDeclaringType() instanceof FastJson 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.
this.getDeclaringType() instanceof FastJson and | |
this.getDeclaringType().getASourceSupertype*() instanceof FastJson 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.
Turns out that these are static methods.
exists(Method m | | ||
this.getMethod() = m and | ||
m.hasName("setSafeMode") and | ||
m.getDeclaringType().hasQualifiedName("com.alibaba.fastjson.parser", "ParserConfig") |
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.
I would add getASourceSupertype*()
to also cover subclasses.
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.
I don't think that's necessary here. I think it's quite unlikely that this will be overridden.
6e9d3e0
to
4be731d
Compare
Rebased to fix merge conflict. |
This cherry-picks the addition of FastJson deserialization sinks from #3674 and adds a check for safe mode sanitization and a change note.