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

Improper Set-Cookie header handling in proxy #582

Closed
flabbergastedbd opened this issue Mar 7, 2016 · 2 comments
Closed

Improper Set-Cookie header handling in proxy #582

flabbergastedbd opened this issue Mar 7, 2016 · 2 comments

Comments

@flabbergastedbd
Copy link
Contributor

Bug

  • Try using owtf proxy to browse to any wordpress admin page.
  • Try logging in. (Invalid credentials will also do)
  • You should see a message on the login page that cookie support seems disabled!!!

Fix

This line

for header, value in list(response.headers.items()):

should be

for header, value in response.headers.get_all():

Verification

  • Repeat the same steps and you should see no such cookies disabled message.
  • Do not send a PR or commit without verifying the fix first!!

Reasoning

  • Servers send multiple Set-Cookie headers
  • When iterated over these headers using items, list of set-cookie header values is joined using comma
  • This format of mentioning multiple cookies in one Set-Cookie header seperated by comma is not recognised by browsers.
  • The ideal way of doing this is by using get_all() method of tornado headers.
@kdexd
Copy link
Contributor

kdexd commented Mar 7, 2016

I will take up upon this issue. The description is quite informative. I am doing a Debian -> Kali transformation and in the middle of setting up Kali. I will first verify it and then send a PR 😄

@kdexd
Copy link
Contributor

kdexd commented Mar 10, 2016

I tested the commit, and in the browser the message did vanish. Though as pointed out by @delta24, I did look up in my /tmp/owtf/proxy.log and find this line:

AttributeError: 'dict' object has no attribute 'get_all'

This seems a bit off.

@viyatb viyatb closed this as completed in 8fac293 Mar 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants