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

Create 4.8.18 Testing for Host Header Injection (OTG-INPVAL-018).md #49

Merged
merged 2 commits into from
May 14, 2019

Conversation

markclayton
Copy link
Contributor

Closes issue #48


# How to Test

Initial testing is as simple as supplying another domain (i.e. attacker.com) into the Host Header field. It is how the web server processes the header value that dictates the impact.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I have read a lot on this matter, this is normal behavior that occurs with virtual hosts as the Host header is used to redirect the users to the vHosts. Don't you think that there should be a clarification on to where this attack is valid? I'm afraid this will entice people to simply send bug reports with Host Header attacks that have no relevant impact and are acting as should.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Host Header field

I'd suggest "the Host header field" (drop the capital on header....throughout the doc, unless it's starting a sentence of course.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah very true. Going to provide additional clarification. Also, i'll drop the capital letter 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes have been made :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review this and let me know if this is adequate, I think i provided a right clarification. Let me know or add whatever you feel is correct.

@@ -0,0 +1,84 @@
# Summary

A web server commonly hosts several web application on the same IP address, referring to each application as a virtual host. In an incoming HTTP request, web servers often dispatch the request to the target virtual host of the value supplied in the Host Header. Without proper validation of the header value, the attacker can supply invalid input to cause the web server to dispatch requests to the first virtual host on the list, cause a 302 redirect to an attacker-controlled domain, perform web cache poisoning, or manipulate password reset functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

several web application

Let's make that applications (plural).

application as a virtual host

Maybe replace as a with via.

Without proper validation of the header value

How about "Without proper validation of the HTTP request Host header value.

input to cause the web server to dispatch requests

I'd suggest adding a colon before the list of potential issues otherwise the sentence runs-on. "...input to cause the web server: To dispatch requests..."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes have been made :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first suggestion was applied to the second application, not the first.


# How to Test

Initial testing is as simple as supplying another domain (i.e. attacker.com) into the Host Header field. It is how the web server processes the header value that dictates the impact.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Host Header field

I'd suggest "the Host header field" (drop the capital on header....throughout the doc, unless it's starting a sentence of course.)

@ThunderSon
Copy link
Collaborator

@markclayton Hello. Any updates on the above?

@markclayton
Copy link
Contributor Author

Hi! Sorry not yet, things have been insane at work here lately. I should have it all knocked out EOD today though. Thanks for the bump.

@ThunderSon
Copy link
Collaborator

No rush! Was just making sure you're still after it. No worries 😄

addresses multiple revision recommendations. Thanks guys for the review, continue to suggest edits as needed.
@markclayton
Copy link
Contributor Author

Sorry for the late response guys! I finally found an opening to make the necessary changes. Please review and continue to provide recommendation and corrections 👍

Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. There are a still a few things outstanding. I'll open a new ticket to track them and if you're feeling up for it you can address them or someone else can tackle it.

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

Successfully merging this pull request may close these issues.

None yet

5 participants