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

netcat may read from invalid file descriptors #143

Closed
leahneukirchen opened this issue Aug 8, 2023 · 0 comments
Closed

netcat may read from invalid file descriptors #143

leahneukirchen opened this issue Aug 8, 2023 · 0 comments

Comments

@leahneukirchen
Copy link

This is a minor issue, but could be avoided easily. When any socket in the poll loop errored, the fd is set to -1 in

if (pfd[n].revents & (POLLERR|POLLNVAL)) {
, however this does not stop later code from calling fillbuf on it, e.g.
ret = fillbuf(pfd[POLL_NETIN].fd, netinbuf,

I suggest readbuf detects fd == -1 and just returns -1 then.

Can be reproduced on Linux with an asynchronous connect that has SO_LINGER set (but probably many ways):

accept4(3, {sa_family=AF_INET, sin_port=htons(39058), sin_addr=inet_addr("127.0.0.1")}, [128 => 16], SOCK_NONBLOCK) = 4
poll([{fd=0, events=POLLIN}, {fd=4, events=0}, {fd=4, events=POLLIN}, {fd=1, events=0}], 4, -1) = 2 ([{fd=4, revents=POLLERR|POLLHUP}, {fd=4, revents=POLLIN|POLLERR|POLLHUP}])
read(-1, 0x7ffe6a792790, 16384)         = -1 EBADF (Bad file descriptor)
@botovq botovq closed this as completed in d4ac551 Aug 14, 2023
bob-beck pushed a commit to openbsd/src that referenced this issue Aug 14, 2023
In case a socket error condition occurs, readwrite() invalidates the
corresponding fd. Later on, readwrite() may still issue a syscall on
it. Avoid that by adding a couple of checks for fd == -1.

Reported and fix suggested by Leah Neukirchen.
Fixes libressl/openbsd#143

"looks right" deraadt
duncan-roe added a commit to duncan-roe/netcat-openbsd that referenced this issue Nov 22, 2023
Also outdent a block of code that was in 1 level too many

OpenBSD commit message was:

    netcat: avoid issuing syscalls on fd -1

    In case a socket error condition occurs, readwrite() invalidates the
    corresponding fd. Later on, readwrite() may still issue a syscall on
    it. Avoid that by adding a couple of checks for fd == -1.

    Reported and fix suggested by Leah Neukirchen.
    Fixes libressl/openbsd#143

    "looks right" deraadt

1 of the 4 code changes was already done (differently). Apply other 3.
allanjude pushed a commit to allanjude/freebsd that referenced this issue Jun 8, 2024
In case a socket error condition occurs, readwrite() invalidates the
corresponding fd. Later on, readwrite() may still issue a syscall on
it. Avoid that by adding a couple of checks for fd == -1.

Reported and fix suggested by Leah Neukirchen.
Fixes libressl/openbsd#143

"looks right" deraadt

(cherry picked from commit 03fa76f835e0954845b206c6f0cadf39975ba82c)
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

No branches or pull requests

1 participant