Unimportant drive-by robot submission

[JLLeitschuh]

Security Vulnerability Fix

This pull request fixes a Temporary File Information Disclosure Vulnerability, which existed in this project.

Preamble

The system temporary directory is shared between all users on most unix-like systems (not MacOS, or Windows). Thus, code interacting with the system temporary directory must be careful about file interactions in this directory, and must ensure that the correct file posix permissions are set.

This PR was generated because a call to File.createTempFile(..) was detected in this repository in a way that makes this project vulnerable to local information disclosure.
With the default uname configuration, File.createTempFile(..) creates a file with the permissions -rw-r--r--. This means that any other user on the system can read the contents of this file.

Impact

Information in this file is visible to other local users, allowing a malicious actor co-resident on the same machine to view potentially sensitive files.

Other Examples

• CVE-2020-15250 - junit-team/junit
• CVE-2021-21364 - swagger-api/swagger-codegen
• CVE-2022-24823 - netty/netty
• CVE-2022-24823 - netty/netty

The Fix

The fix has been to convert the logic above to use the following API that was introduced in Java 1.7.

File tmpDir = Files.createTempFile("temp dir").toFile();

The API both creates the file securely, ie. with a random, non-conflicting name, with file permissions that only allow the currently executing user to read or write the contents of this file.
By default, Files.createTempFile("temp dir") will create a file with the permissions -rw-------, which only allows the user that created the file to view/write the file contents.

➡️ Vulnerability Disclosure ⬅️

👋 Vulnerability disclosure is a super important part of the vulnerability handling process and should not be skipped! This may be completely new to you, and that's okay, I'm here to assist!

First question, do we need to perform vulnerability disclosure? It depends!

1. Is the vulnerable code only in tests or example code? No disclosure required!
2. Is the vulnerable code in code shipped to your end users? Vulnerability disclosure is probably required!

Vulnerability Disclosure How-To

You have a few options options to perform vulnerability disclosure. However, I'd like to suggest the following 2 options:

1. Request a CVE number from GitHub by creating a repository-level GitHub Security Advisory. This has the advantage that, if you provide sufficient information, GitHub will automatically generate Dependabot alerts for your downstream consumers, resolving this vulnerability more quickly.
2. Reach out to the team at Snyk to assist with CVE issuance. They can be reached at the Snyk's Disclosure Email.

Detecting this and Future Vulnerabilities

This vulnerability was automatically detected by GitHub's CodeQL using this CodeQL Query.

You can automatically detect future vulnerabilities like this by enabling the free (for open-source) GitHub Action.

I'm not an employee of GitHub, I'm simply an open-source security researcher.

Source

This contribution was automatically generated with an OpenRewrite refactoring recipe, which was lovingly hand crafted to bring this security fix to your repository.

The source code that generated this PR can be found here:
SecureTempFileCreation

Opting-Out

If you'd like to opt-out of future automated security vulnerability fixes like this, please consider adding a file called
.github/GH-ROBOTS.txt to your repository with the line:

User-agent: JLLeitschuh/security-research
Disallow: *

This bot will respect the ROBOTS.txt format for future contributions.

Alternatively, if this project is no longer actively maintained, consider archiving the repository.

CLA Requirements

This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.

It is unlikely that I'll be able to directly sign CLAs. However, all contributed commits are already automatically signed-off.

The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin
(see https://developercertificate.org/ for more information).

- Git Commit Signoff documentation

If signing your organization's CLA is a strict-requirement for merging this contribution, please feel free to close this PR.

Sponsorship & Support

This contribution is sponsored by HUMAN Security Inc. and the new Dan Kaminsky Fellowship, a fellowship created to celebrate Dan's memory and legacy by funding open-source work that makes the world a better (and more secure) place.

This PR was generated by Moderne, a free-for-open source SaaS offering that uses format-preserving AST transformations to fix bugs, standardize code style, apply best practices, migrate library versions, and fix common security vulnerabilities at scale.

Tracking

All PR's generated as part of this fix are tracked here: JLLeitschuh/security-research#18

[elharo]

Have you reviewed this yourself? E.g can you explain the impact in context? Automated PRs are rarely helpful.

[JLLeitschuh]

I've reviewed this vulnerability in example projects, yes... For this particular projects use case, no I have not. Doing so for over 1k projects by-hand is, unfortunately impractical

[elharo]

I am very tired of having static analysis tools run on repos and filing bugs or PRs that have no understanding of context. They rarely reveal real bugs, and mostly just waste developer time. This is one such. On further investigation, this is not a security problem, and I'm closing the PR.

[JLLeitschuh]

I am very tired of having static analysis tools run on repos and filing bugs or PRs that have no understanding of context.

This is exactly what I don't file bugs. They do waste your time. I figured an easily mergeable pull request would be more palatable by maintainers because, in the worst case it's a real vulnerability, and in the best case, it's still a valid security hardening.

I've gotten significantly more push back when the fix is exclusive to tests. I think I'll update the recipes to only generate test fixes when production code is also modified.

[garydgregory]

I asked this user to stop creating PRs against Apache Commons many times to no avail. Imagine having PRs against test code labeled "SECURITY" over and over... I finally had to add a gh-robots.txt file to most Commons repositories. Not only is this FUD but if this were indeed a SECURITY issue, filling it here is against our most basic and documented responsible reporting process.

[slawekjaranowski]

So I ask again @JLLeitschuh - can you exclude Maven projects from your boot PR?

Of course all proper PR with jira issue, reporting for ASF security team and other contributions are welcome.

[michael-o]

So I ask again @JLLeitschuh - can you exclude Maven projects from your boot PR?

Of course all proper PR with jira issue, reporting for ASF security team and other contributions are welcome.

He won't do this because it is too much work. We should just add a robot exclude.

[JLLeitschuh]

I finally had to add a gh-robots.txt file to most Commons repositories.

If it is the will and decision of the Apache Commons PMC, I will of course respect this.

Not only is this FUD but if this were indeed a SECURITY issue, filling it here is against our most basic and documented responsible reporting process.

Would automated emails with patch files attached be preferred?

If you'd like to discuss this more in-person feel free to setup some time on my calendar this week or next to discuss your concerns. I'm more than happy to discuss this more.

https://calendly.com/jonathan-leitschuh-at-dan-kaminsky-fellowship/30min

[bmarwell]

I finally had to add a gh-robots.txt file to most Commons repositories.

If it is the will and decision of the Apache Commons PMC, I will of course respect this.

Not only is this FUD but if this were indeed a SECURITY issue, filling it here is against our most basic and documented responsible reporting process.

1. This is not Apache commons, this is part of the Apache maven project.
2. While of course contributions are welcome and refusing those would need PMC attention, you are already ignoring the request to not report security issues publicly: https://www.apache.org/security/#reporting-a-vulnerability

I must admit it never came to my mind to suggest a file .well-known/security.txt (for explanation, see: https://developer.okta.com/blog/2021/10/19/intro-security-txt). However, before most security researchers report vulnerabilities, it is already well-known and common to look WHERE to report them first.
Please adopt the same behaviour.

Thanks.

[JLLeitschuh]

Would automated emails with patch files attached be preferred?

I reiterate this question. Is this what the ASF process would prefer?

This is not Apache commons, this is part of the Apache maven project.

I agree. But the comment I was responding to was from @garydgregory who stated he had personally placed these files in all of the Apache Commons files. I presume this was done with the blessing of the ASF PMC, and I would of course respect it if that were the case.

you are already ignoring the request to not report security issues publicly

How do you propose I do this at scale? When reporting hundreds or thousands of security vulnerabilities across OSS, I am only one person. How do you propose I attempt to follow the policies of every org I report to? How would you solve this problem?

The problem is, there are a lot of common ways that vulnerabilities appear in open source. I've opened 165 pull requests to fix Zip-Slip across the java OSS ecosystem. Zip-slip is well recognized as a fairly critical security vulnerability when that code is reachable by an external actor.

Other similar vulnerabilities impact our industry writ-large. My knowledge of security vulnerabilities doesn't scale well if I'm expected to triage and report each vulnerability 1 at a time. In an ideal world, I completely agree this would be how we'd want to solve this problem. But it's impractical.

According to GitHub, there is 1 security researcher to every 500 developers. We are heaviy outnumbered. I believe, although there will be false positives, that providing an easily mergeable fix is the best way to actually see these widespread common security vulnerabilities eliminated in the most efficient manner from the industry. The goal of all of these fixes is to have them either be valid security fixes, or at worse, valid security hardening worth merging regardless.

The feedback regarding pure-test security PRs has forced me to reasess a bit, and I'll be improving the recipe logic to prevent pure-test fixes from being created with FUD language.

Anyone in this thread is more than welcome to grab some time on my calendar to discuss this in person. I welcome the feedback and the ability to discuss this with you all more moving forward.

https://calendly.com/jonathan-leitschuh-at-dan-kaminsky-fellowship/30min

[elharo]

How do you propose I do this at scale? When reporting hundreds or thousands of security vulnerabilities across OSS, I am only one person. How do you propose I attempt to follow the policies of every org I report to? How would you solve this problem?

Patient: Doctor, it hurts when I bang my head against the wall.
Doctor: Simple, stop banging your head against the wall.

You are not reporting hundreds or thousands of security vulnerabilities across OSS. You are running an automated code analyzer that spams many repos and maintainers with low quality, non-bugs. This is not helpful. Any actual problems the tool uncovers are completely lost in a sea of false positives. If the bugs the tool finds are not worth your time to investigate, why do you think the bugs will be worth other people's time?

[garydgregory]

Brilliant explanation @elharo , my sentiments exactly.

[michael-o]

Gosh, I enjoyed every single minute of this tragedy on stage with my popcorn...

[olamy]

🍿

[JLLeitschuh]

I've clearly upset and frustrated several of you through this process, and for that I'm sorry.

You are running an automated code analyzer that spams many repos and maintainers with low quality, non-bugs. This is not helpful.

Given your view of the world, and the PRs that you all have received to-date, I can understand this response and why that would be upsetting. This is not representative of the general experience of maintainers I've had across the board.

I'm working on compiling some of the data from the various campaigns and the over 5,000 automated pull requests I've issued to-date to provide accurate merge/rejection-counts and potentially also some sentiment analysis on the responses from the maintainers.

To provide a smaller snapshot from my previous campaigns that I do have data for.

I generated 1,596 pull requests to fix the use of HTTP (instead of HTTPS) to resolve dependencies in maven POM files back in 2020. This has had a 40% merge rate as of 2022.

For this particular campaign, local temporary file information disclosure, 58 PRs have been issued to-date, two CVEs will be issued so far:

• [SECURITY] Fix Temporary File Information Disclosure Vulnerability  cowtowncoder/java-merge-sort#21
• [SECURITY] Fix Temporary File Information Disclosure Vulnerability  james-mime4j#81

I suspect more will come out of this work as well.

Give me a bit more time to come up with better statistics. I have some other bulk PR campaigns I engaged in, but my merge rate vs close rate statistics are out-of-date, and I don't want to misrepresent the current state of the world. I'd also like to collect the stats on the number of CVEs that have been issued from this work as well.

[JLLeitschuh]

If any of you have feedback on the messaging I'm using in particular. All of the PR messages are completely open source and I would accept PRs to reduce the perceived FUD you see I'm spreading through the messages being sent out.

The messaging used is per-campaign as found in these sub directories.

https://github.com/JLLeitschuh/security-research/tree/main/bulk-pr-generation