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

Add trace header to http-request-headers-fields-large.txt #963

Merged
merged 3 commits into from
Feb 13, 2024

Conversation

denandz
Copy link
Contributor

@denandz denandz commented Jan 16, 2024

Adds the X-Trace header to the large header fields list.

@molangning
Copy link
Contributor

#939 Is a improvement of the checker scripts but it hasn’t merged yet. Can I commit that fix instead?

@denandz
Copy link
Contributor Author

denandz commented Jan 16, 2024

#939 Is a improvement of the checker scripts but it hasn’t merged yet. Can I commit that fix instead?

Looks like your pull still has the newline bug, using for line in contents.split(b'\n'): instead of for line in contents.splitline(False):. Your proposed changes don't look like they'll fix the erroneous checker fail that happened here.

@molangning
Copy link
Contributor

I’m still fixing it, would this suffice?

    for line in contents.splitlines():
        line=line.strip()

@molangning
Copy link
Contributor

molangning commented Jan 16, 2024

The purpose of the checker was to detect empty lines and new lines at the end of the files.

@denandz
Copy link
Contributor Author

denandz commented Jan 16, 2024

You don't need the line.strip(), since splitlines() will get rid of the trailing newline. splitlines(False) is the default, you don't need to supply it but I figure it doesn't hurt to be clear.

>>> str = "hello\nmolangning\nhow's it\ngoing?\n"
>>> str.splitlines()
['hello', 'molangning', "how's it", 'going?']
>>> str.splitlines(False)
['hello', 'molangning', "how's it", 'going?']
>>> str.splitlines(True)
['hello\n', 'molangning\n', "how's it\n", 'going?\n']
>>> 

@molangning
Copy link
Contributor

molangning commented Jan 16, 2024

There is a possible case where instead of an empty line there is a line with just whitespace.

[“hello”, “   “, “”]

the second element should be flagged as well because it just contains whitespace

line.strip removes that whitespace so that b’ ‘ becomes b’’

@denandz
Copy link
Contributor Author

denandz commented Jan 16, 2024

Respectfully, I disagree here. A chain of whitespace of varying length is a valid fuzzing payload that I'd expect to exist in the fuzzing wordlists.

@molangning
Copy link
Contributor

I agree with you, and so that’s why it outputs a warning and not an error. It’s not a requirement but if there isn’t supposed to be a empty line in the list the author can catch it early before having it merged.

Should I add another case where it warns about the whitespace?

@molangning
Copy link
Contributor

molangning commented Jan 16, 2024

I added some more changes

for line in contents.splitlines():
if not line:
print_normal("[!] Warning: %s has an empty entry on line %i!"%(i,counter))
print_warn(i,counter)
pass_status=False
continue
if not line.strip():
print_normal("[!] Warning: %s has an whitespace only entry on line %i!"%(i,counter))
print_warn(i,counter)
pass_status=False
continue

@g0tmi1k
Copy link
Collaborator

g0tmi1k commented Feb 13, 2024

Thanks @denandz

@g0tmi1k g0tmi1k merged commit d37927a into danielmiessler:master Feb 13, 2024
1 check passed
@g0tmi1k g0tmi1k self-assigned this Feb 13, 2024
@g0tmi1k g0tmi1k added the enhancement Enhancement label Feb 13, 2024
This was referenced Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants