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.

Sign up for GitHub

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

Jump to bottom

socket.accept() might raise ECONNABORTED crashing the server on FreeBSD #105

Closed
giampaolo opened this issue May 28, 2014 · 8 comments
Closed

socket.accept() might raise ECONNABORTED crashing the server on FreeBSD #105

giampaolo opened this issue May 28, 2014 · 8 comments
Labels
bug Component-Library imported imported from old googlecode site and very likely outdated OpSys-BSD Priority-High Security

Comments

@giampaolo
Copy link
Owner

From [email protected] on April 02, 2009 17:47:40

What steps will reproduce the problem?  
1. Run a ftp server.
2. Use nmap like this: nmap -sT -p 21 locahost
3. The following exception is raised:

Traceback (most recent call last):
  File "ftpserver.py", line 3263, in <module>
    test()
  File "ftpserver.py", line 3260, in test
    ftpd.serve_forever()
  File "ftpserver.py", line 3140, in serve_forever
    poll_fun(timeout, map)
  File "/usr/local/lib/python2.6/asyncore.py", line 141, in poll
    read(obj)
  File "/usr/local/lib/python2.6/asyncore.py", line 78, in read
    obj.handle_error()
  File "/usr/local/lib/python2.6/asyncore.py", line 74, in read
    obj.handle_read_event()
  File "/usr/local/lib/python2.6/asyncore.py", line 408, in handle_read_event
    self.handle_accept()
  File "ftpserver.py", line 3157, in handle_accept
    sock, addr = self.accept()
  File "/usr/local/lib/python2.6/asyncore.py", line 339, in accept
    conn, addr = self.socket.accept()
  File "/usr/local/lib/python2.6/socket.py", line 195, in accept
    sock, addr = self._sock.accept()
socket.error: [Errno 53] Software caused connection abort


The only system where this is reproducible so far is FreeBSD.

Original issue: http://code.google.com/p/pyftpdlib/issues/detail?id=105
@giampaolo
Copy link
Owner Author

From [email protected] on April 02, 2009 08:59:15

Summary: socket.accept() might raise ECONNABORTED crashing the server on FreeBSD

@giampaolo
Copy link
Owner Author

From [email protected] on April 02, 2009 09:49:27

Fixed as r563 .

@giampaolo
Copy link
Owner Author

From [email protected] on April 02, 2009 10:46:38

Status: Finished

@giampaolo
Copy link
Owner Author

From [email protected] on April 04, 2009 06:42:45

Labels: Crash

@giampaolo
Copy link
Owner Author

From [email protected] on August 29, 2009 10:34:02

Status: FixedInSVN

@giampaolo
Copy link
Owner Author

From [email protected] on September 13, 2009 13:56:15

Status: Fixed

@giampaolo
Copy link
Owner Author

From [email protected] on September 13, 2009 14:01:52

This is now fixed and included as part of 0.5.2 version.

@giampaolo
Copy link
Owner Author

From g.rodola on November 05, 2010 19:03:31

Added a test case which covers this but in r749 .

grembo added a commit to grembo/tonic that referenced this issue Aug 17, 2024
@grembo
Prevent server from exiting on ECONNABORTED
0223f51 
On FreeBSD, accept can fail with ECONNABORTED, which means that
"A connection arrived, but it was on the listen queue"
(see `man 2 accept`).

Without this patch, a server without the tls feature  exits returing 0
in this case, which makes it vulnerable not only to intentional denial
of service, but also to unintentional crashes, e.g., by haproxy TCP
health checks.

The problem can be reproduced on any FreeBSD system by running the
tonic "helloworld" example (without feature TLS) and then sending a
packet using nmap:

    cd examples
    cargo run --bin helloworld-server --no-default-features &
    nmap -sT -p 50051 -6 ::1
    # server exits

When running the example with the feature tls enabled, it won't exit
(as the tls event loop in tonic/src/transport/server/incoming.rs
handles errors gracefully):

    cd examples
    cargo run --bin helloworld-server --no-default-features \
      features=tls &
    nmap -sT -p 50051 -6 ::1
    # server keeps running

This patch is not optimal - it removes some generic error parameters
to gain access to `std::io::Error::kind()`. The logic itself should be
sound.

See also:

- https://man.freebsd.org/cgi/man.cgi?accept(2)
  Accept man page
- giampaolo/pyftpdlib#105
  giampaolo/pyftpdlib@0f82232
  Basically the same issue (and its fix) in another project
grembo added a commit to grembo/tonic that referenced this issue Aug 18, 2024
@grembo
Prevent server from exiting on ECONNABORTED
6c399ca 
On FreeBSD, accept can fail with ECONNABORTED, which means that
"A connection arrived, but it was on the listen queue"
(see `man 2 accept`).

Without this patch, a server without the tls feature  exits returing 0
in this case, which makes it vulnerable not only to intentional denial
of service, but also to unintentional crashes, e.g., by haproxy TCP
health checks.

The problem can be reproduced on any FreeBSD system by running the
tonic "helloworld" example (without feature TLS) and then sending a
packet using nmap:

    cd examples
    cargo run --bin helloworld-server --no-default-features &
    nmap -sT -p 50051 -6 ::1
    # server exits

When running the example with the feature tls enabled, it won't exit
(as the tls event loop in tonic/src/transport/server/incoming.rs
handles errors gracefully):

    cd examples
    cargo run --bin helloworld-server --no-default-features \
      features=tls &
    nmap -sT -p 50051 -6 ::1
    # server keeps running

This patch is not optimal - it removes some generic error parameters
to gain access to `std::io::Error::kind()`. The logic itself should be
sound.

See also:

- https://man.freebsd.org/cgi/man.cgi?accept(2)
  Accept man page
- giampaolo/pyftpdlib#105
  giampaolo/pyftpdlib@0f82232
  Basically the same issue (and its fix) in another project
@grembo grembo mentioned this issue Aug 18, 2024
Merged
github-merge-queue bot pushed a commit to hyperium/tonic that referenced this issue Aug 20, 2024
@grembo
Prevent server from exiting on ECONNABORTED (#1874)
b4c3518 
* Prevent server from exiting on ECONNABORTED

On FreeBSD, accept can fail with ECONNABORTED, which means that
"A connection arrived, but it was on the listen queue"
(see `man 2 accept`).

Without this patch, a server without the tls feature  exits returing 0
in this case, which makes it vulnerable not only to intentional denial
of service, but also to unintentional crashes, e.g., by haproxy TCP
health checks.

The problem can be reproduced on any FreeBSD system by running the
tonic "helloworld" example (without feature TLS) and then sending a
packet using nmap:

    cd examples
    cargo run --bin helloworld-server --no-default-features &
    nmap -sT -p 50051 -6 ::1
    # server exits

When running the example with the feature tls enabled, it won't exit
(as the tls event loop in tonic/src/transport/server/incoming.rs
handles errors gracefully):

    cd examples
    cargo run --bin helloworld-server --no-default-features \
      features=tls &
    nmap -sT -p 50051 -6 ::1
    # server keeps running

This patch is not optimal - it removes some generic error parameters
to gain access to `std::io::Error::kind()`. The logic itself should be
sound.

See also:

- https://man.freebsd.org/cgi/man.cgi?accept(2)
  Accept man page
- giampaolo/pyftpdlib#105
  giampaolo/pyftpdlib@0f82232
  Basically the same issue (and its fix) in another project

* Handle ECONNABORTED without breaking APIs

This simplifies the previous patch a lot. The string search
is ugly (also not 100% if it's needed or if we could handle
errors like in the TLS enabled accept loop).

* Next iteration based on feedback

* More review feedback

* Only use std::io if needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Component-Library imported imported from old googlecode site and very likely outdated OpSys-BSD Priority-High Security
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@giampaolo