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

Fix for CVE-2017-15133 TCP DOS #631

Merged
merged 1 commit into from
Jan 25, 2018
Merged

Fix for CVE-2017-15133 TCP DOS #631

merged 1 commit into from
Jan 25, 2018

Conversation

miekg
Copy link
Owner

@miekg miekg commented Jan 25, 2018

serveTCP calls reader.ReadTCP in the accept loop rather than in
the per-connection goroutine. If an attacker opens a connection
and leaves it idle, this will block the accept loop until the
connection times out (2s by default). During this time no other
incoming connections will succeed, preventing legitimate queries
from being answered.

This commit moves the call to reader.ReadTCP into the per-connection
goroutine. It also adds a missing call to Close whose absence allowed
file-descirptors to leak in select cases.

This attack and fix have no impact on serving UDP queries.

serveTCP calls reader.ReadTCP in the accept loop rather than in
the per-connection goroutine. If an attacker opens a connection
and leaves it idle, this will block the accept loop until the
connection times out (2s by default). During this time no other
incoming connections will succeed, preventing legitimate queries
from being answered.

This commit moves the call to reader.ReadTCP into the per-connection
goroutine. It also adds a missing call to Close whose absence allowed
file-descirptors to leak in select cases.

This attack and fix have no impact on serving UDP queries.
@miekg
Copy link
Owner Author

miekg commented Jan 25, 2018

Fixes #631

@codecov-io
Copy link

codecov-io commented Jan 25, 2018

Codecov Report

Merging #631 into master will decrease coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #631      +/-   ##
==========================================
- Coverage   57.89%   57.89%   -0.01%     
==========================================
  Files          37       37              
  Lines        9987     9989       +2     
==========================================
+ Hits         5782     5783       +1     
- Misses       3156     3157       +1     
  Partials     1049     1049
Impacted Files Coverage Δ
server.go 58.14% <50%> (-0.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 862243b...ee6a9d6. Read the comment docs.

@miekg miekg merged commit 43913f2 into master Jan 25, 2018
@miekg miekg deleted the cve branch January 25, 2018 10:36
@miekg miekg mentioned this pull request Jan 25, 2018
tianon added a commit to tianon/rawdns that referenced this pull request Jan 25, 2018
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