OpenIDC / mod_auth_openidc

OpenID Certified™ OpenID Connect Relying Party implementation for Apache HTTP Server 2.x
Apache License 2.0
990 stars 327 forks source link

Vulnerable to HTTP Response Split in logout? #68

Closed davidbernick closed 9 years ago

davidbernick commented 9 years ago

Hi all!

I was doing a scan (IBM AppScan to be precise) and

So if a user is successfully phished to click on a logout link (in an email or website or whatever) they could insert headers into the redirect which can poison the cache, right?

GET /callback/?logout=https%3A%2F%2Fsite.siteplace.com%2Flogin%2Flogin%2F%0d%0aAppScanHeader:%20AppScanValue%2f1%2e2%2d3%0d%0aSecondAppScanHeader:%20whatever

HTTP/1.1 302 Found
Date: Fri, 29 May 2015 17:24:27 GMT
Server: Apache
Location: https://site.siteplace.com/login/login/
AppScanHeader: AppScanValue/1.2-3     <-- THIS
SecondAppScanHeader: whatever    <-- AND THIS
Content-Length: 379
Content-Type: text/html; charset=iso-8859-1
Set-Cookie: mod_auth_openidc_session=;Path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT;Domain=siteplace.com;Secure

Is there a way to have a parameter sanitized somehow? A list of exact urls for redirection?

zandbelt commented 9 years ago

I do see that the logout mechanism creates an open redirect so it would be better to restrict the URLs by default to the domain the web server runs on (optionally tightened up further by a config option).

I don't see how the headers would poison the cache. Headers can be added from user agents regardless of the logout mechanism. Can you elaborate?

davidbernick commented 9 years ago

From what I understand (and I could be wrong), inserting headers can lead to these two kinds of attacks. I would call these "medium" attacks, not critical or show-stoppers. I don't think this is a bug or a really big deal, but it's something: https://www.owasp.org/index.php/HTTP_Response_Splitting https://www.owasp.org/index.php/Cache_Poisoning

zandbelt commented 9 years ago

ok, I did not inspect the request URL closely enough, I see what you mean now; all 3 attacks (open redirect, response splitting, cache poisoning) result from the fact that the value of the logout parameter in not validated, I will add that

zandbelt commented 9 years ago

please check this branch: https://github.com/pingidentity/mod_auth_openidc/commits/fix-logout-validation

zandbelt commented 9 years ago

in the master branch now since https://github.com/pingidentity/mod_auth_openidc/commit/15e936c484a44388dcd0ef3a478d6c6d8264feea