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

improves http body decoding and enforces max length #295

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

bwireman
Copy link
Contributor

@bwireman bwireman commented Feb 2, 2021

improves http body decoding and enforces max length

How to Test

echo "www.qq.com" | ./zgrab2 http -p 443 --use-https --max-size=64 --with-body-size | jq .

Notes & Caveats

I decided the initial read should still be limited and that we should just re-enforce length afterwards, but I can see the argument for not doing that

qq.com is a good example because it decodes correctly even though it isn't certain

@bwireman bwireman force-pushed the bw/improve-http-body-decoding branch from 5ed7568 to ea5fb9d Compare February 2, 2021 18:06
decoder := encoder.NewDecoder()

//"windows-1252" is the default value and will likely not decode correctly
if certain || encoding != "windows-1252" {
Copy link
Member

Choose a reason for hiding this comment

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

Why won't windows-1252 decode correctly? Did you run into specific issues with that encoding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I should have explained this better. windows-1252 is what determineEncoding returns when it has no idea and is causing issues like the one we saw this morning. So unless it's certain I don't want to use it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

An edge case would be a detection of windows-1252 earlier than the default (via prescan, I guess). But given the returns of charset, I guess there is no other way to know that this is the default rather than computed guess (other than manually re-implementing charset.DetermineEncoding). So your solution is probably best.

@bwireman bwireman merged commit d9ed4f1 into master Feb 3, 2021
@bwireman bwireman deleted the bw/improve-http-body-decoding branch February 3, 2021 15:56
constantinsander pushed a commit to COMSYS/quic-zgrab2 that referenced this pull request Sep 11, 2023
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.

2 participants