Skip to content

[java] Merged with 3665 (https://github.com/github/codeql/pull/3665) #3674

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

Closed
wants to merge 1 commit into from
Closed

Conversation

haby0
Copy link
Contributor

@haby0 haby0 commented Jun 10, 2020

[java] Merged with 3665 (#3665)

@p-
Copy link
Contributor

p- commented Jun 10, 2020

@haby0 This query as it stands now does not actually detect potential Fastjson RCE vulnerabilities. Besides parsing JSON, Autotyping needs to be enabled (either globally or locally).

@haby0
Copy link
Contributor Author

haby0 commented Jun 11, 2020

@p- When writing codeql rules, it is considered to add local detection of autoType, but autoType may be bypassed.
fastjson <=1.2.68 RCE vulnerability, official announcement: https://github.com/alibaba/fastjson/wiki/security_update_20200601

@haby0
Copy link
Contributor Author

haby0 commented Jun 11, 2020

@p- I think I can increase the detection of autoType in the codeql rules, but there will be some underreporting, because autoType has the risk of being bypassed. And I began to try to detect the set and get methods of dependent packages.

@p-
Copy link
Contributor

p- commented Jun 11, 2020

@haby0 Oh wow, that's interesting... Makes it harder to write a good query for this.

@aschackmull
Copy link
Contributor

The additions here related to Spring are by now supported out-of-the-box - comprehensive Spring support was added in #3653.
The addition of the FastJson deserialization sink looks good, but is missing a check for safe mode. I've cherry-picked the FastJson contribution and cleaned everything up along with the added safe mode check in a new PR: #4427

@aschackmull aschackmull closed this Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants