[Java]: CWE-601 Spring url redirection detect

haby0 commented 3 years ago

Query

Link to pull request with your CodeQL query:

Relevant PR: https://github.com/github/codeql/pull/5844

CVE ID(s)

List the CVE ID(s) associated with this vulnerability. GitHub will automatically link CVE IDs to the GitHub Advisory Database.

Report

Describe the vulnerability. Provide any information you think will help GitHub assess the impact your query has on the open source community.

[x] Are you planning to discuss this vulnerability submission publicly? (Blog Post, social networks, etc). We would love to have you spread the word about the good work you are doing

Result(s)

Provide at least one useful result found by your query, on some revision of a real project.

ghsecuritylab commented 3 years ago

Your submission is now in status SecLab review.

For information, the evaluation workflow is the following: CodeQL initial assessment > SecLab review > CodeQL review > SecLab finalize > Pay > Closed

ghsecuritylab commented 3 years ago

Your submission is now in status CodeQL review.

For information, the evaluation workflow is the following: CodeQL initial assessment > SecLab review > CodeQL review > SecLab finalize > Pay > Closed

ghsecuritylab commented 3 years ago

Your submission is now in status SecLab finalize.

For information, the evaluation workflow is the following: CodeQL initial assessment > SecLab review > CodeQL review > SecLab finalize > Pay > Closed

haby0 commented 3 years ago

This query appears to be a duplicate of my work in github/codeql#4214. The initial draft before merge included all of the sinks along with the flow logic. The result set of the my initial query should cover all of results detected by this one.

However, per the conversation here, it was deemed appropriate if the query was split into two separate issues. There is already an open redirect query for Java here. The part removed from my initial query is extremely similar to the logic of this query and was to be added to the open redirect query.

I was planning on taking this up once the query was merged on March 2, I was planning on taking it up. However, due to COVID-19, couldn't make much progress. Most of the changes in this PR in my opinion can be achieved with simple refactoring of existing code. It would be interesting to see security-lab's stand on this issue.

After your comment, I read this PR in full (not followed before), this is your initial [PR] (https://github.com/github/codeql/commit/54b463130831a956554eefc14d494ee344cf06b9).

Here are a few points I want to say:

The original intention of the PR you submitted was to detect Spring View operational vulnerabilities, not Spring-specific redirection vulnerabilities.
Your initial query did not consider RedirectView object/ModelAndView object/"redirect:" prefix redirection problem.
Your initial PR, use the return expression of SpringControllerMappingMethod as a sink. I have to admit that the sink here is indeed you Part of the sink. But your sink is too extensive.
This is the commit you started to add "redirect:", but it was excluded. The redirection vulnerability is mentioned here by @pwntester

Also, I don't know what this has to do with COVID-19? Please advise.

pwntester commented 3 years ago

Hi @porcupineyhairs @haby0, After reviewing the facts, we don't find reasons to flag this issue as a collision. Six months have passed since I identified those cases as View Manipulation FPs and suggested moving them to a specific Open redirect query. We don't expect program participants to read all the PR comments and removed code from all the program submissions to know if someone else may or may not work on a specific query. I hope this clears things up, let me know if you have any further questions

ghsecuritylab commented 3 years ago

Your submission is now in status Pay.

For information, the evaluation workflow is the following: CodeQL initial assessment > SecLab review > CodeQL review > SecLab finalize > Pay > Closed

xcorail commented 3 years ago

Created Hackerone report 1204658 for bounty 306369 : [366] [Java]: CWE-601 Spring url redirection detect

ghsecuritylab commented 3 years ago

Your submission is now in status Closed.

For information, the evaluation workflow is the following: CodeQL initial assessment > SecLab review > CodeQL review > SecLab finalize > Pay > Closed

github / securitylab