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

Lowered the timeout from one minute to a default of 20 seconds #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Gijutsu
Copy link

@Gijutsu Gijutsu commented May 10, 2016

which in my tests proved to be around 10 times the required time to get a response from a patched or vulnerable server - but it of course depends on your network latency.

@@ -52,7 +52,8 @@ func main() {
http.HandleFunc("/check/", CheckServer)
s := &http.Server{
Addr: os.Args[1],
ReadTimeout: 10 * time.Second,
ReadTimeout: 1 * time.Second,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change these? It's totally reasonable for a connection to stall for 1 second.

Copy link
Author

@Gijutsu Gijutsu May 13, 2016

Choose a reason for hiding this comment

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

I agree, this change is not necessary to lower the timeout when running against a host that is not responding. The only change that is actually needed is "deadline := time.Now().Add(2 * time.Second)" I'll amend my pull request to reflect this.

@benjojo
Copy link
Contributor

benjojo commented May 13, 2016

Cool, Could you squash? It may be a good idea to add this as a flag, since 2 seconds is in the realms of a TCP connection missing something, and in some cases you really want to avoid a false positive, and other times you are less caring :)

@FiloSottile
Copy link
Owner

I'm unlikely to put in a default lower than 20 seconds. Some of us have crappy connections :) Please make this a flag if you want to be able to configure something lower.

by adding the command line flags timeout and address while keeping
backwards compability by making the usage of the flags optional.
@Gijutsu Gijutsu changed the title Lowered the timeout from one minute to a maximum of 2 seconds, Lowered the timeout from one minute to a default of 20 seconds May 13, 2016
@Gijutsu
Copy link
Author

Gijutsu commented May 13, 2016

Thanks for the feedback. I can see the benefit of making this a flag, so I have converted the PR to use flags while keeping backwards compability by making the usage of the flags optional.

Example usage:
./CVE-2016-2107 --help
Usage of ./CVE-2016-2107:
-address string
Server to test (default "127.0.0.1")
-timeout int
Connection timeout in seconds (default 20)

time ./CVE-2016-2107 8.8.8.8
2016/05/13 23:19:55 dial tcp 8.8.8.8:443: i/o timeout

real 0m20.002s
user 0m0.000s
sys 0m0.000s

./CVE-2016-2107 -address filippo.io -timeout 2
2016/05/13 23:22:06 Vulnerable: false

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.

3 participants