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

fastjson 经常出漏洞问题 建议替换fastjson或者增加其他json库适配 #1522

Open
woshioosm opened this issue Jun 5, 2020 · 9 comments
Labels
kind/discussion For further discussion

Comments

@woshioosm
Copy link

source的读取是可以自定义json库读取的 但是在sentinel-transport-common中是依赖fastjson的 这里能否替代掉 或者可适配其他json库

@Warkeeper
Copy link

Strongly agree.
Since the metrics API is enabled by default, it's easy to construct a critical JSON message and send it to a application which uses sentinel. So if sentinel strongly depends a vulnerable json libary like FastJson, it may cause the application easy to attack.
For now, my team uses javassist to mock fastjson-releated class to deceive the Sentinel while completely excludes the fastjson, and uses other json lib as the implementation. But in the long term, I think sentinel should replace the dependency of fastjson with some kind of adapter.

@sczyh30 sczyh30 added the kind/discussion For further discussion label Jun 5, 2020
@sczyh30
Copy link
Member

sczyh30 commented Jun 9, 2020

@jasonjoo2010 @cdfive @linlinisme @zhaoyuguang Any thoughts?

@jasonjoo2010
Copy link
Collaborator

Im ok about that.
Or we can just disable insecure features like autotype.

As i knew before autotype is disabled by default and we also can use our own ParserConfig to make sure it running in secured mode.

So is there any other insecure feature or other reason that it should be abandoned?

@Warkeeper
Copy link

According to this doc https://github.com/alibaba/fastjson/wiki/security_update_20200601 , it seems like even if we enable safeMode to disable autoType, the security still cannot be guaranteed.

配置safeMode后,无论白名单和黑名单,都不支持autoType,可一定程度上缓解反序列化Gadgets类变种攻击

@jasonjoo2010
Copy link
Collaborator

According to this doc https://github.com/alibaba/fastjson/wiki/security_update_20200601 , it seems like even if we enable safeMode to disable autoType, the security still cannot be guaranteed.

配置safeMode后,无论白名单和黑名单,都不支持autoType,可一定程度上缓解反序列化Gadgets类变种攻击

Oh, i see and thank you for your information.

We indeed can make adaptions to kinds of json parsers like fastjson, jackson, gson, etc.
But notice that not only fastjson but also jackson i remembered also has vulnerable versions.
So adaptions may not really solve the security issue but give ability to users migrating to another.
Users may also use a vulnerable version of parser but they don't provide public entrance like HTTP. They are safe in such scenario.

So my another proposal besides adaptions is that is it necessary to alter a lite version like jackson / fastjson preserving basic serialization / deserialization only?

Adaptions is straightforward and good i should say.

@jasonjoo2010
Copy link
Collaborator

So any update?

@jasonjoo2010
Copy link
Collaborator

So is it necessary to recall the PR from the long river of the time?
Please let me know if so and I will solve the conflicts.

@545653
Copy link

545653 commented Jul 1, 2021

So is it necessary to recall the PR from the long river of the time?
Please let me know if so and I will solve the conflicts.

Yes!

@sczyh30 sczyh30 modified the milestones: 1.8.2, 1.8.3 Jul 5, 2021
@sczyh30 sczyh30 removed this from the 1.8.3 milestone Dec 31, 2021
@dylan-tao
Copy link

So is it necessary to recall the PR from the long river of the time? Please let me know if so and I will solve the conflicts.

Yes, some customers are already repulsed by the passive mass upgrade that FASTJSON brings,thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion For further discussion
Projects
None yet
Development

No branches or pull requests

6 participants