Skip to content

Spring MVC controller browser downloads "f.txt" #4220

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
TianhuaRainbow opened this issue Oct 17, 2015 · 21 comments
Closed

Spring MVC controller browser downloads "f.txt" #4220

TianhuaRainbow opened this issue Oct 17, 2015 · 21 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@TianhuaRainbow
Copy link

The maven version 1.2.7:

@RequestMapping("/a.html")
@ResponseBody
public String a() {
    return "abcd";
}

As shown above:
If visit *.html,it will return "f.txt" to download.

@wilkinsona
Copy link
Member

Sorry, but I don't think I understand the problem you're having. Where's f.txt coming from? If you'd like us to help, please provide a sample project that reproduces the problem.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Oct 17, 2015
@vpavic
Copy link
Contributor

vpavic commented Oct 17, 2015

f.txt is coming from Content-Disposition response header. This is a consequence of fix for RFD Attack in Spring Framework.

Simple Hello World app reproduces the issue:

@RestController
@SpringBootApplication
public class DemoApplication {

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @RequestMapping("/hello")
    public String home() {
        return "Hello World!";
    }

}

curl -I http://localhost:8080/hello.html returns:

HTTP/1.1 200 OK
Server: Apache-Coyote/1.1
Content-Disposition: attachment;filename=f.txt
Content-Type: text/html;charset=UTF-8
Content-Length: 12

Same happens with request for a missing resource, curl -I http://localhost:8080/world.html returns:

HTTP/1.1 404 Not Found
Server: Apache-Coyote/1.1
Content-Disposition: attachment;filename=f.txt
Content-Type: application/json;charset=UTF-8
Content-Length: 114

@snicoll
Copy link
Member

snicoll commented Oct 17, 2015

ping @rstoyanchev

@snicoll snicoll added the type: blocker An issue that is blocking us from releasing label Oct 19, 2015
@snicoll snicoll added this to the 1.2.8 milestone Oct 19, 2015
@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Oct 19, 2015
@wilkinsona
Copy link
Member

This breaks accessing an individual metric using a browser. In 1.2.6.RELEASE, pointing a browser to http://localhost:8080/metrics/mem.free displays the value of the metric in the browser window. In 1.2.7.RELEASE, it downloads a file named f.txt.

@rstoyanchev
Copy link
Contributor

I've created SPR-13587 to consider an improvement for the metrics case and also SPR-13588 to consider an improvement for the error dispatch case mentioned by @vpavic.

As for the @ResponseBody with String return value case, we explicitly chose not to whitelist "html". The StringHttpMessageConverter and ByteArrayHttpMessageConverter are the only converters configured very openly out of the box (*/*) to accept any requested content type. This means in many cases simply adding ".html" will cause the method to render as "text/html". If the response contains some user input, like a URI variable, request parameter, or other, this can easily result in XSS or RFD attack. This can be surprising if the controller method returns String with the intention to render "text/plain".

An application can explicitly register "html" is an extension for content negotiation purposes. It's the application's responsibility then to sanitize any input included in the response. Spring Boot can help to make this easy to configure through an application property but it shouldn't be hard otherwise either.

I'm also wondering about the use cases that lead to rendering HTML with the StringHttpMessageConverter. If you could please share some feedback on that.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Oct 26, 2015
@snicoll
Copy link
Member

snicoll commented Oct 26, 2015

@TianhuaRainbow any insight?

@bartolom
Copy link

bartolom commented Nov 1, 2015

If you decide to not whitelist ".html" in @RespnseMapping by default due to security concerns, I'm ok with this as I like the fact that Java people take security more serious then other communities.

BUT, please try to make at least the necessary configuration steps more obvious. I think that most people that run a web application don't do it to serve suspicious "forced f.txt downloads". If you within the Spring Framework already know that this response will cause this maybe you could

  • Write out a logging line at INFO level saying somethink like "You are about to cause a response that will create a f.txt file, you can do this and that..."
  • Maybe throwing an exception is even better
  • Improve documentation that is really necessray to configure/whitelist
  • Write a Spring getting started guide that points this out. (As a side note http://spring.io/guides/gs/serving-web-content/ uses a templating setup, so it's different).

I have a setup where the response with the .html is part of a pipeline that pre-arranges Polymer Web Components at the server side which are imported as fragments with a CORS call from a Polymer application. Maybe in 2015 still an exotic setup but surely a growing niche. I was looking at so many places before I found this out.

It's odd that you can still write a helloWorld.json web-server based on Spring Boot in 140 characters but for serving a helloWorld.html you need to add a call to RequestMappingHandlerMapping.setUseRegisteredSuffixPatternMatch(). For a long time Spring user thats OK, and I can live with it, but for people new to the technology?

I'm really not an expert on Spring internals but maybe you could use the "produces" parameter. In my example I used

@RequestMapping(value = "/with.html", produces = MediaType.TEXT_HTML_VALUE)

So I as a developer stated that I really wanted to serve html files for this controller method call.

Or you can force me to state that I'm responsible to be aware of the risk with a isSanitized or similar:

@RequestMapping(value = "/with.html", isSanitized=true)

(A better name can surely be found, isSafe=true, iKnowAboutTheRisk=true, ... )

@rstoyanchev
Copy link
Contributor

@bartolom thanks for the thorough feedback! I've created https://jira.spring.io/browse/SPR-13629. We shouldn't need any additional flags like isSanitized. The presence of ".html" in the mapping or a produces condition for text/html should be sufficient.

The key concern otherwise is that unsuspecting text-rendering controller methods can be coerced into rendering HTML with very bad consequences. The risk is very real.

@dsyer
Copy link
Member

dsyer commented Nov 4, 2015

I'm going to put in a whitelist configuration option. It seems like it's going to be super common for users to want to do this for any non-standard file content that they don't want to see come down as "f.txt". Assuming the new feature should go in 1.3 (because it sort of belongs in the spring.resources.* bucket - any arguments the other way? I suppose maybe we should support it in 1.2.8 as well (therefore a different property path in server.*)?

@dsyer dsyer modified the milestones: 1.3.0, 1.2.8 Nov 4, 2015
@dsyer dsyer added type: enhancement A general enhancement and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 4, 2015
dsyer added a commit that referenced this issue Nov 5, 2015
Allows users to configure "allowed" file extensions for controller
mappings, so that browsers will not switch to downloading "f.txt"
(part of the recent RFD attack fixes in Spring MVC).

See gh-4220
@dsyer
Copy link
Member

dsyer commented Nov 5, 2015

User can now add spring.mvc.mediaTypes in config file, as well as in WebMvcConfigurer if desired (e.g. spring.mvc.mediaTypes.yml=text/yaml) in 1.2.8 and 1.3.0.

@dsyer dsyer changed the title err return f.txt Spring MVC controller browser downloads "f.txt" Nov 5, 2015
@dsyer dsyer closed this as completed Nov 5, 2015
@wilkinsona
Copy link
Member

-1 on the new property. We discussed and rejected adding it in #4240. What's changed?

@snicoll
Copy link
Member

snicoll commented Nov 5, 2015

You said 1.2.8 but it's only master right?

@dsyer
Copy link
Member

dsyer commented Nov 5, 2015

It's in 1.2.x as well (commit 124574e).

@wilkinsona what changed is I realized how common it is going to be to need to do this, and provide a way for users to extend the defaults. Even a really basic use case with a controller that returns a content type not covered by the new whitelist is quite a head scratcher for users and I don't want to have to write Java code to support something as simple and common as a mime type mapping.

@dsyer dsyer modified the milestones: 1.2.8, 1.3.0 Nov 5, 2015
@wilkinsona
Copy link
Member

Thanks, @dsyer. I'm still not sure that a property in Boot is the best answer. Shouldn't it be handled in Spring Framework? Perhaps it already has under SPR-13629? Assuming that it fully addresses the problem, using a request mapping with an explicit file extension or a produces clause feels like a better solution to me, not least because it isn't a Boot-specific solution to a broader problem.

@dsyer
Copy link
Member

dsyer commented Nov 5, 2015

An explicit produces clause or file extension can't be used for endpoints that return static resources (for instance) of different types. The "mediaTypes" property of the ContentNegotiationManager is pretty much ideal for that use case.

SPR-13629 included a sensible change to consider media types that are "known" to be also "safe". So I guess in a sense this change is complementary with that.

snicoll added a commit that referenced this issue Nov 5, 2015
Add missing documentation

See gh-4220
@philwebb philwebb reopened this Nov 5, 2015
@philwebb philwebb closed this as completed Nov 5, 2015
alanvdam added a commit to OpenConext-Attic/OpenConext-csa that referenced this issue Dec 3, 2015
This broke because spring also sets the content-disposition header
to prevent a RFD Attack.

- https://pivotal.io/security/cve-2015-5211
- spring-projects/spring-boot#4220
@wyyl1
Copy link

wyyl1 commented Sep 27, 2016

You can use the Spring 4.2.5.RELEASE version.
Before I use the Spring 4.2.2.RELEASE version will download f.txt file

@stevesgit
Copy link

thx @wyyl1, this is work for me . When I add mvc:message-converters tag in dispatcher-servlet.xml ,also appear this problem.

@blop
Copy link

blop commented Oct 17, 2018

Not sure how this should work now.
Is spring webmvc designed only for HTTP clients that happen to be a browser ?

I get this Content-Disposition: inline;filename=f.txt which I cannot remove, very annoying.

I have an endpoint containing a dot in the path, and spring mvc assumes this is a file, althought the method is annotated with "produces = MediaType.APPLICATION_JSON_UTF8_VALUE"

Is there any clean way to disable this behaviour, or disable the content negotiation entirely ?

@wilkinsona
Copy link
Member

@blop Can you please ask questions on Stack Overflow or Gitter? As mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements.

@blop
Copy link

blop commented Oct 17, 2018

@wilkinsona Should I create an new issue then to discuss this as an enhancement ?

@wilkinsona
Copy link
Member

It looked to me like you were asking a question:

Is there any clean way to disable this behaviour, or disable the content negotiation entirely ?

If so, it belongs on Stack Overflow.

If you think you've exhausted all possible ways of getting the behaviour that you want, then you should open a Spring Framework issue as it's Spring MVC that's adding the Content-Disposition header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests