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

Spring Unvalidated Redirect Detector #289

Merged
merged 3 commits into from Apr 5, 2017

Conversation

johnhawes
Copy link
Contributor

This adds a detector for unvalidated redirects in Spring MVC web applications. The detector looks for a view prefix of "redirect:", which instructs the controller to perform a redirect to a specified URL. The detector only looks at classes with "RequestMapping" annotations. This should limit inspection to controllers and abstract classes that define endpoints.

@h3xstream
Copy link
Member

h3xstream commented Apr 4, 2017

The log of travis-ci was unavailable when I last checked. Now it is available : https://travis-ci.org/find-sec-bugs/find-sec-bugs/builds/216934937

The build failure is related to missing LGPL license header.

[WARNING] Missing header in: /home/travis/build/find-sec-bugs/find-sec-bugs/plugin/src/main/java/com/h3xstream/findsecbugs/spring/SpringUnvalidatedRedirectDetector.java
[WARNING] Missing header in: /home/travis/build/find-sec-bugs/find-sec-bugs/plugin/src/test/java/com/h3xstream/findsecbugs/spring/SpringUnvalidatedRedirectDetectorTest.java

public class SpringUnvalidatedRedirectController {

@RequestMapping("/redirect1")
public String redirect1(@RequestParam("url") String url) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the return value is ModelAndView?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should detect redirects for String and ModelAndView return values. Essentially, the detector looks for string concatenation using the "redirect:" prefix in Spring controllers. This could lead to false positives, but there should be limited false negatives. I have updated the tests to confirm it will detect ModelAndViews and added LGPL license headers.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 87.145% when pulling 4a643d0 on johnhawes:master into afa7ec6 on find-sec-bugs:master.

@h3xstream h3xstream merged commit 7229065 into find-sec-bugs:master Apr 5, 2017
@h3xstream h3xstream added this to the version-1.7.0 milestone Jul 24, 2017
@h3xstream h3xstream added the enhancement New feature or improvement to existing detector. label Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing detector.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants