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

[Java]: CWE-073 - File path injection with the JFinal framework #527

Closed
1 of 2 tasks
luchua-bc opened this issue Jan 23, 2022 · 10 comments
Closed
1 of 2 tasks

[Java]: CWE-073 - File path injection with the JFinal framework #527

luchua-bc opened this issue Jan 23, 2022 · 10 comments
Labels
All For One Submissions to the All for One, One for All bounty

Comments

@luchua-bc
Copy link

Query PR

github/codeql#7712

Language

Java

CVE(s) ID list

CVE-2021-44093

  • A Remote Command Execution vulnerability on the background in zrlog 2.2.2, at the upload avatar function, could bypass the original limit, upload the JSP file to get a WebShell

CVE-2021-40639

  • Improper access control in Jfinal CMS 5.1.0 allows attackers to access sensitive information via /classes/conf/db.properties&config=filemanager.config.js

CWE

CWE-073: External Control of File Name or Path

Report

External Control of File Name or Path, also called File Path Injection, is a common attack and injection attack is listed as one of the top attacks in OWASP Top Ten 2021.

Loading files based on unvalidated user-input may cause file information disclosure and uploading files with unvalidated file types to an arbitrary directory may lead to Remote Command Execution (RCE).

JFinal is a widely used Web + ORM framework, which has 1.4K forks and 3.2k stars on GitHub. More introduction can be found at JFinal Tutorial. Multiple CWEs have been submitted for File Path Injection attack associated with this framework.

This query detects unsafe file loading/downloading operations in code repositories that consume this framework. It models JFinal input methods as remote flow source using the source model CSV format. It creates a separate PathSanitizer library so that the library can be promoted as a shared lib that can be used by other queries as well. It reduces FPs by pruning the sink and testing with both real projects on GitHub and test cases customized for this query.

Are you planning to discuss this vulnerability submission publicly? (Blog Post, social networks, etc).

  • Yes
  • No

Blog post link

No response

@luchua-bc luchua-bc added the All For One Submissions to the All for One, One for All bounty label Jan 23, 2022
@ghsecuritylab
Copy link
Collaborator

Your submission is now in status Test run.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

@ghsecuritylab
Copy link
Collaborator

Your submission is now in status Results analysis.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

@kevinbackhouse
Copy link
Contributor

@luchua-bc: I have confirmed that this query finds CVE-2021-44093, but I haven't able to confirm that it finds CVE-2021-44093. These are the query results for jflyfox/jfinal_cms:

https://lgtm.com/query/1921658844250522458/

I don't think any of those results are the vulnerability, are they?

@luchua-bc
Copy link
Author

Thanks @kevinbackhouse for reviewing this PR.

For CVE-2021-40639 with jflyfox/jfinal_cms, it's a false negative. We just need to add the following single taint step to the query as shown in https://lgtm.com/query/8743969029633232154/ to detect the issue with FileManager:

/** Taint model related to `jflyfox`. */
private class RequestFlowStep extends SummaryModelCsv {
  override predicate row(string row) {
    row =
      [
        "com.jflyfox.modules.filemanager;FileManager;true;sanitize;;;Argument[0];ReturnValue;taint"
      ]
  }
}

As it's jflyfox specific, which is not generic enough, I didn't include it in the submitted query.

Four results already found by the query are valid as well although the ConfigManager one is concatenated with a file name/path suffix, whose exploitation is limited in scope. I deployed the latest version to my local server and proved the file path injection vulnerability can be exploited through a payload uploadPath=../../../. I've submitted a new issue jflyfox/jfinal_cms#31 to jflyfox, which demoed how the file path injection vulnerability with com.jflyfox.system.file.UploadController can be exploited step by step.

@ghsecuritylab
Copy link
Collaborator

Your submission is now in status Query review.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

@ghsecuritylab
Copy link
Collaborator

Your submission is now in status Final decision.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

@ghsecuritylab
Copy link
Collaborator

Your submission is now in status Pay.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

@xcorail
Copy link
Contributor

xcorail commented Feb 17, 2022

Created Hackerone report 1483918 for bounty 369596 : [527] [Java]: CWE-073 - File path injection with the JFinal framework

@xcorail xcorail closed this as completed Feb 17, 2022
@ghsecuritylab
Copy link
Collaborator

Your submission is now in status Closed.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

@luchua-bc
Copy link
Author

Thanks @xcorail for the quick turn-around and the bounty:-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
All For One Submissions to the All for One, One for All bounty
Projects
None yet
Development

No branches or pull requests

4 participants