Nmap Development mailing list archives

[PATCH] Ncat --broker & --ssl disconnects broken


From: Kris Katterjohn <katterjohn () gmail com>
Date: Tue, 30 Jun 2009 17:50:20 -0500

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hey everyone,

While messing around with the second patch for the ncat --send-only behavior,
I stumbled upon a bug in Ncat's broker mode when using SSL.

Basically, there are no checks against the return value of SSL_read happening
in broker mode, so when EOF and error conditions occur, things mess up.  I
don't have time to track down the exact codepath of what happens where after
this occurs, but it's obvious it's bad (it appears to be some sort of infinite
loop--maybe select() keeps returning for the underlying socket but nothing
happens?)

The checks exist in the same function as SSL_read; however, they're in the
wrong spot so it's just dead code now.  There's a block of code that handles
what should happen for these conditions on both regular sockets and SSL
sockets, but they only run if this happens on a regular socket.

I've attached a patch to move these checks to the right place.  All seems well
in my brief testing (and it's a very simple patch).  Note that these changes
are in the same spot as some changes in my --send-only patch so they probably
won't apply cleanly together unfortunately.

Cheers,
Kris Katterjohn

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iQIcBAEBAgAGBQJKSpasAAoJEEQxgFs5kUfuDF8P/3trDTzp0GYwLRzlrwtUkdPb
KN577htmehV0odfRKa3gEUwHOK7+yMlXVSuncYivncJwgeW2GusXMw7yUpSA+13C
ZCh3cOhH93a9iGKqn7Pw9fL8qC5IPNfjHAUQJOHEpNmNKxpQDkyjJYhV5qIEmmdg
gvXogmVeLCtXz1sw1qeIwuMr4tqAMCR/rV2xOwkH14RjemExzuYehxIdHHOVUTe2
OyYCd0wpDMBJKfvh9mIlqJN/1AhNpYLOuqruhqu4jrMAl+JTXI4dPAuViZnj3L9M
DUbH8IQ1u2skBesJlAVsgzSMcvCWFLxSBGkL4lbWl7RFHUOxFEzim4mJ29WQzE09
7tiDswt6og3nwoBDDcnjc/UusAnVMLEXJtmIxO3e9m9n7oM6BVD5bVdUbuvfnj4u
HPiObwpDEWx9AJwlwIqAH1hBA9JnVP4IH5E6xrRhtZ1inF2yT7xTWFDYhXIJkbjN
X15H1mb59ew//A1XHdwTySvdJ8wfcBjaPrmaFz4b2EhKncb2skn+Vi9OxdMhna23
yHYWTn8h1N+RU+zO+fJUU0fsQ89TP8X2xQ1uibviYHoso7Czo1XxAVP6jthmtmIm
MMYXeBccTp4nmV2STunNt+vaUcOC+KH+fy2G+nclx9rmibkPmv+sAmP1Zr3MZXN3
YHu3eIyphTOAs2WgDsKK
=SQ6u
-----END PGP SIGNATURE-----
Index: ncat_broker.c
===================================================================
--- ncat_broker.c       (revision 13984)
+++ ncat_broker.c       (working copy)
@@ -330,36 +330,37 @@
         /* From a connected socket, not stdin. */
         nbytes = recv(recv_fd, buf, sizeof(buf), 0);
 
-        if (nbytes <= 0) {
-            if (o.debug)
-                logdebug("Closing connection.\n");
+        /* If we received from something other than stdin, and --send-only was
+           given, do no further processing. */
+        if (nbytes > 0 && o.sendonly)
+            return;
+    }
 
+    /* Handle EOF or error from network */
+    if (nbytes <= 0) {
+        if (o.debug)
+            logdebug("Closing connection.\n");
+
 #ifdef HAVE_OPENSSL
-            if (o.ssl && fdn->ssl) {
-                if (nbytes == 0)
-                    SSL_shutdown(fdn->ssl);
-                SSL_free(fdn->ssl);
-            }
+        if (o.ssl && fdn->ssl) {
+            if (nbytes == 0)
+                SSL_shutdown(fdn->ssl);
+            SSL_free(fdn->ssl);
+        }
 #endif
 
-            close(recv_fd);
-            FD_CLR(recv_fd, &master);
-            rm_fd(&fdlist, recv_fd);
+        close(recv_fd);
+        FD_CLR(recv_fd, &master);
+        rm_fd(&fdlist, recv_fd);
 
-            conn_count--;
-            if (conn_count == 0)
-                FD_CLR(STDIN_FILENO, &master);
+        conn_count--;
+        if (conn_count == 0)
+            FD_CLR(STDIN_FILENO, &master);
 
-            if (o.chat)
-                chat_announce_disconnect(recv_fd);
+        if (o.chat)
+            chat_announce_disconnect(recv_fd);
 
-            return;
-        }
-
-        /* If we received from something other than stdin, and --send-only was
-           given, do no further processing. */
-        if (o.sendonly)
-            return;
+        return;
     }
 
     if (o.debug > 1)

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org

Current thread: