spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.72k stars 5.87k forks source link

HTTP Host header attack #4310

Closed fraenku closed 5 years ago

fraenku commented 7 years ago

The class UrlUtils is using the methodgetServerName()of the HttpServletRequest. This method indeed is not secure since it could be manipulated through the host-header

See also: http://www.skeletonscribe.net/2013/05/practical-http-host-header-attacks.html) or: https://find-sec-bugs.github.io/bugs.htm#SERVLET_SERVER_NAME

See also: https://github.com/ESAPI/esapi-java-legacy/blob/develop/src/main/java/org/owasp/esapi/filters/SecurityWrapperRequest.java for a list of potential request headers which are manipulable.

ibaskine commented 6 years ago

This issue is detected by Burp Scanner, which makes it difficult selling products based on Spring Security to security conscious customers. Are there any plans fixing this issue or is there any workaround?

fraenku commented 6 years ago

Some progress on this? issue is open since April... The issue above leaded to an attack where an URL in an e-mail could be manipulated (which can be easily used for phising-attacks)

ibaskine commented 6 years ago

Just wander. Can I use code snippet below for getting server name?

new java.net.URI(request.getRequestURL().toString()).getHost()

rady66 commented 5 years ago

What is the progress here? Could hardly believe action has not been taken for such security issue almost two years.

gjoseph commented 5 years ago

FWIW SourceClear reports this as a vulnerability (https://www.sourceclear.com/vulnerability-database/security/incorrect-headers-handling/java/sid-20558) - any chance this could get patched up soon?

jzheaux commented 5 years ago

Since the Host header is untrusted information, we should likely consider whitelisting as a solution. StrictHttpFirewall is the most suitable, so I'd propose adding a method StrictHttpFirewall#setAllowedHostnames(Predicate), which would ensure the hostname in HttpServletRequest is a known good value.

The application would need to set this value like so:

void configure(WebSecurity web) {
    StrictHttpFirewall firewall = new StrictHttpFirewall();
    firewall.setAllowedHostnames(hostname -> hostname.equals("example.org"));
    web
        .httpFirewall(firewall);
}

With this solution, UrlUtils would still invoke request.getServerName(), but it would be a known good value at that point.

@fraenku would this address the issue?

avodonosov commented 5 years ago

Does anyone know a use case where spring-security itself opens a vulnerability due to the Host header manipulation?

Assuming my application code doesn't use UrlUtils for anything sensitive to attacks from the description, is it still dangerous to just use spring-security?

@fraenku , @ibaskine , @rady66 , @gjoseph , @jzheaux

jzheaux commented 5 years ago

Does anyone know a use case

This ticket doesn't describe any attacks. If anyone does know of one, I'd encourage that it be reported responsibly to security@pivotal.io, not here.

is it still dangerous to just use spring-security

No piece of software is perfectly secure, @avodonosov, but there is quite a difference between that and describing a software library as "dangerous". Can you explain a bit more of what you mean here?

avodonosov commented 5 years ago

@jzheaux , I mean is there a real vulnerability if I simply use spring-security functionality, without taking UrlUtils result myself and sending it somewhere. I just mean is it really exploitable, or it's just a general note that the Host is accessed by code, so it smells vulnerable, but no-one can exploit it.

jzheaux commented 5 years ago

I mean is there a real vulnerability

There are always insecure ways to use things, but the main principle here is to not use untrusted information in a security decision.

This ticket was not about closing a reported vuln but instead about adding functionality that allows applications to whitelist the Host header, making it a known-good value. The default behavior of Spring Security is still to allow any Host header, so its default security position hasn't changed either way by merging this PR.

[is it] just a general note that the Host is accessed by code

UrlUtils#buildFullRequestUrl uses request.getServerName, and so if you are using buildFullRequestUrl as part of a security decision, and you haven't previously validated the Host header, then you may be vulnerable somehow as that violates the principle. Spring Security doesn't use UrlUtils#buildFullRequestUrl in any known-vulnerable ways.

It's hard to know the myriad circumstances under which others are calling this method, which is why understanding the security principle is more important.

avodonosov commented 5 years ago

Thank you, @jzheaux

gjoseph commented 5 years ago

This ticket doesn't describe any attacks. If anyone does know of one, I'd encourage that it be reported responsibly to security@pivotal.io, not here.

@jzheaux I think what I'm still a bit confused about is this issue was linked against a Sourceclear vulnerability (https://www.sourceclear.com/vulnerability-database/security/incorrect-headers-handling/java/sid-20558) - it may be a red herring, but neither side is telling us how one relates to the other, or where the relation even comes from. I figured with this SourceClear vuln being reported, the Pivotal folks would already be aware?

jzheaux commented 5 years ago

it may be a red herring, but neither side is telling us how one relates to the other

@gjoseph Sorry that I don't have an update for you on this yet, though I am in the process of reaching out to SourceClear to get more information from them.

avodonosov commented 5 years ago

@ibaskine, you say it was detected by Burp Scanner. Can you provide a link, or more details?

This means there is a real use case - Burp Suite is not a static code analyzer, it performs queries on a real alive system. trying various exploits (shell commands in request parameters, SQL; various XSS techniques).

So Burp Suite has sent a malicious Host header and detected insecure use (e.g. received the value passed embedded literally in HTML response).

jzheaux commented 5 years ago

I'd simply reiterate here that if there is a known exploit that individuals please report it responsibly to security@pivotal.io with steps to reproduce and not outline exploit details here on GitHub.

red herring... neither side is telling us how one relates to the other

The reporter has the burden of proof. We can only guess at the things SourceClear may or may not be referring to. (As I said earlier, I've reached out to them myself and am awaiting a response.)

Automated tools have the natural drawback of false positives. The actual description from the report casts a wide, generic-sounding net, and until we receive more detailed information, it's quite a bit of guesswork.

As a matter of cleanup, I've opened a ticket to remove one usage of the Host header, but again, there's no telling why an automated tool complained.

To give that report more weight, and to expedite any related fixes, we'd need more detail about the problems - I'd invite those in the community who are paying SourceClear to create these reports to lean on them for support in this matter.

rwinch commented 5 years ago

To add to what @jzheaux said:

We take bugs seriously, but without having a clear explanation of the problem it is difficult to fix the issue. If this is an issue, please report the issue with details on how to reproduce. If the issue is a security related issue, please report via the instructions at https://pivotal.io/security

avodonosov commented 5 years ago

@rwinch , @jzheaux , I'm not demanding you to provide a proof or an explanation. Other way around, I'm trying to find out the specifics myself. @ibaskine mentioned it's found by Burp Scanner, I'm asking for the details of the failure, it may help to understand the real issue.

Of course, if that's an exploitable vulnerability, better to report to vendor privately first (https://pivotal.io/security)

rwinch commented 5 years ago

@avodonosov

I can understand how you misunderstood our responses (they weren't written as well as I would have hoped). So to start...I'm sorry if our responses were misinterpreted.

We know you are trying to help and we appreciate that. Thanks for the reply and taking the time to try and make Spring Security better. I understand you are looking for the specifics and we appreciate you are trying to help us solve the problem. Unfortunately, we don't have specifics either.

The unfortunate reality is that automated tools report lots of false positives. If the Spring Security team chased every automated report without a concrete problem stated, we would never get anything done for the project.

For that reason, we recommend that you ask the projects reporting the issue for details (since they are the ones reporting the issue they should have details). If something concrete is found, please let us know and we will be happy to help. Thank you again for taking the time try and make Spring Security better.

peeyushsurolia commented 5 years ago

it may be a red herring, but neither side is telling us how one relates to the other

@gjoseph Sorry that I don't have an update for you on this yet, though I am in the process of reaching out to SourceClear to get more information from them.

@jzheaux I observed that this issue : https://www.sourceclear.com/vulnerability-database/security/incorrect-headers-handling/java/sid-20558 not reported on 5.1.6.RELEASE of spring-security-web but 4.X series is vunerable.

I am currently using 4.X version and can't switch to latest version at this point of time, Are you still in discussion to get details from SourceClear?

jzheaux commented 5 years ago

Yes, @peeyushsurolia, I am still awaiting their response.

snoopysecurity commented 4 years ago

I noticed this has been patched now, shouldn't there be an advisory for this within https://tanzu.vmware.com/security?

jzheaux commented 4 years ago

Thanks, @snoopysecurity, for double-checking. Given that these changes don't resolve any CVE, I'm not sure what we'd report there.