Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

CRLF http header injection #40

Merged
merged 10 commits into from
Jul 14, 2016
Merged

CRLF http header injection #40

merged 10 commits into from
Jul 14, 2016

Conversation

hone
Copy link
Member

@hone hone commented Jul 13, 2016

@mattreduce reported that the static buildpack is vulnerable to CRLF injection when you try to visit it without HTTPS and you've not received the STS header yet (initial redirect to force SSL).

You can see it adds a header to its response that you inject in your own request:

cURL output for a request with header injection:

$ curl -i http://conway-hi-poc.herokuapp.com/%0d%0aset-cookie:%20test=123;

HTTP/1.1 301 Moved Permanently
Connection: keep-alive
Server: nginx
Date: Wed, 06 Jul 2016 23:44:00 GMT
Content-Type: text/html
Content-Length: 178
Location: https://conway-hi-poc.herokuapp.com/
Set-Cookie: test=123
Via: 1.1 vegur
<html>
<head><title>301 Moved Permanently</title></head>
<body bgcolor="white">
<center><h1>301 Moved Permanently</h1></center>
<hr><center>nginx</center>
</body>
</html>

This patch fixes it as well as add SSL termination to the test infrastructure so we can test against SSL.

@mattreduce
Copy link

It looks like the build is failing due to some Docker-related issues, but these changes look good to me 👍

@hone
Copy link
Member Author

hone commented Jul 13, 2016

@mattreduce yeah, CircleCI operates docker a bit different than locally, so I need to massage it and workaround some issues. I'll get that fixed up before I merge.

@hone hone force-pushed the crlf_http_header_injection branch from 4004147 to 5a510f7 Compare July 14, 2016 01:24
hone added 4 commits July 13, 2016 20:27
Fixes the following:

curl -i http://conway-hi-poc.herokuapp.com/%0d%0aset-cookie:%20test=123;

HTTP/1.1 301 Moved Permanently
Connection: keep-alive
Server: nginx
Date: Wed, 06 Jul 2016 23:44:00 GMT
Content-Type: text/html
Content-Length: 178
Location: https://conway-hi-poc.herokuapp.com/
Set-Cookie: test=123
Via: 1.1 vegur
<html>
<head><title>301 Moved Permanently</title></head>
<body bgcolor="white">
<center><h1>301 Moved Permanently</h1></center>
<hr><center>nginx</center>
</body>
</html>
@hone hone force-pushed the crlf_http_header_injection branch from 5a510f7 to 6c113d3 Compare July 14, 2016 01:32
hone added 3 commits July 13, 2016 20:46
Circle CI does not allow deletion of containers:

Cannot destroy container
6f30ab3de4f445d4fe47059d68d9798d40f5f66b9a1120dcfd130c0cab378cb5: Driver
btrfs failed to remove root filesystem
6f30ab3de4f445d4fe47059d68d9798d40f5f66b9a1120dcfd130c0cab378cb5: Failed
to destroy btrfs snapshot: operation not permitted
@hone hone force-pushed the crlf_http_header_injection branch from d2f575f to c378c66 Compare July 14, 2016 01:46
@hone hone merged commit 4d4cf34 into master Jul 14, 2016
@hone hone deleted the crlf_http_header_injection branch July 14, 2016 01:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants