Re: [Lynx-dev] SOCK5 crash+fix

[Steffen Nurpmeso]

Hello.

I was the original author of the $SOCKS5_PROXY patch, but for one
Thomas Dickey changed the C enormously, and i also have forgotten
anything about it, but you noting it made me look.

which had a 
Gisle Vanem wrote in
 <8906d293-f9ff-3d11-89d7-4bdeb944afd0@gmail.com>:
 |While trying Lynx with Tor's SOCK5 proxy:
 |   lynx -dump -socks5_proxy=localhost:9050
 |       https://www.bbcweb3hytmzhn5d532owbu6oqadra5z3ar726vq5kgwwn6aucdccrad\
 |       .onion/
 |
 |(the BBC Homepage), I got a strange crash for this code:
 |    socks5_protocol = HTSprintf0(NULL,
 |                                 gettext("(for %s at %s) SOCKS5"),
 |                                 protocol, socks5_host);
 |
 |A NULL-ptr read which I fail to understand.

Ok you mean that function may return NULL in out-of-memory and
thus this condition should be checked.
Hm, but HTAlloc() bails fatally in this condition?

  if (ptr == 0)
      outofmem(__FILE__, "HTAlloc");

 |But simply replacing with:
 |    char socks5_buf [1000];
 |    ...
 |    snprintf(socks5_buf, sizeof(socks5_buf),
 |             gettext("(for %s at %s) SOCKS5"), protocol, socks5_host);
 |    protocol = socks5_buf;

..So this i do not buy.

 |plus some other patches to HTTCP.c (attached), Lynx+Tor
 |works on Windows-10. The diff is against the 2.9.0dev.10
 |version. Latest I believe (?)
 ...
 |--- orig/WWW/Library/Implementation/HTTCP.c 2021-06-09 01:44:43
 |+++ WWW/Library/Implementation/HTTCP.c      2022-03-10 14:11:59
 |@@ -1828,9 +1828,9 @@
 |     char *socks5_host = NULL;
 |     unsigned socks5_host_len = 0;
 |     int socks5_port;
 |+    char socks5_buf [1000];
 |     const char *socks5_orig_url;
 |     char *socks5_new_url = NULL;
 |-    char *socks5_protocol = NULL;
 |     int status = HT_OK;
 |     char *line = NULL;
 |     char *p1 = NULL;
 |@@ -1888,10 +1888,9 @@
 |  HTSACat(&socks5_new_url, socks5_proxy);
 |  url = socks5_new_url;
 |
 |-     socks5_protocol = HTSprintf0(NULL,
 |-                                  gettext("(for %s at %s) SOCKS5"),
 |-                                  protocol, socks5_host);
 |-     protocol = socks5_protocol;
 |+     snprintf(socks5_buf, sizeof(socks5_buf),
 |+              gettext("(for %s at %s) SOCKS5"), protocol, socks5_host);
 |+     protocol = socks5_buf;
 |}
 | #ifndef INET6
 |     /*

With the above said, this is unrelated and worse or worse :)

 |@@ -2032,8 +2031,10 @@
 |   *                      write  service  procedure.  This will be
 |   *                      the normal case.
 |   */
 |+     CTRACE((tfp, "connect(): status: %d, SOCK_ERRNO: %d\n", status, \
 |SOCKET_ERRNO));
 |+
 |  if ((status < 0) &&
 |-         (SOCKET_ERRNO == EINPROGRESS
 |+         (SOCKET_ERRNO == EINPROGRESS || SOCKET_ERRNO == 112
 | #ifdef EAGAIN
 ||| SOCKET_ERRNO == EAGAIN
 | #endif

I do not know what 112 is, but i have no idea of Windows, what
i know was 3.* and 95 B.  You are surely right.

 |@@ -2091,7 +2092,7 @@
 |    * If we suspend, then it is possible that select will be
 |    * interrupted.  Allow for this possibility.  - JED
 |    */
 |-             if ((ret == -1) && (errno == EINTR))
 |+             if ((ret == -1) && (SOCKET_ERRNO == EINTR))
 |       continue;
 |
 | #ifdef SOCKET_DEBUG_TRACE

This looks right to me given that SOCKER_ERRNO != errno there.

 |@@ -2273,7 +2281,7 @@
 |  pbuf[0] = 0x05;             /* VER: protocol version: X'05' */
 |  pbuf[1] = 0x01;             /* NMETHODS: 1 */
 |  pbuf[2] = 0x00;             /* METHOD: X'00' NO AUTHENTICATION REQUIRED \
 |  */
 |-     if (write(*s, pbuf, 3) != 3) {
 |+     if (NETWRITE(*s, pbuf, 3) != 3) {

Interesting!  This ... seems right then too?
I did not know this!

  ...

The below patch against [d84fd6821970236a99d0d38b1df98c74463cec02]
aka v2-9-0dev_10a, does it work for you too?
(Note i have not even compile-tested it, but .. should!!!)

diff --git a/WWW/Library/Implementation/HTTCP.c 
b/WWW/Library/Implementation/HTTCP.c
index c4d648acde..99f4620f2f 100644
--- a/WWW/Library/Implementation/HTTCP.c
+++ b/WWW/Library/Implementation/HTTCP.c
@@ -2036,6 +2036,9 @@ int HTDoConnect(const char *url,
            (SOCKET_ERRNO == EINPROGRESS
 #ifdef EAGAIN
             || SOCKET_ERRNO == EAGAIN
+#endif
+#ifdef _WINDOWS
+            || SOCKET_ERRNO == 112
 #endif
            )) {
            struct timeval select_timeout;
@@ -2091,7 +2094,7 @@ int HTDoConnect(const char *url,
                 * If we suspend, then it is possible that select will be
                 * interrupted.  Allow for this possibility.  - JED
                 */
-               if ((ret == -1) && (errno == EINTR))
+               if ((ret == -1) && (SOCKET_ERRNO == EINTR))
                    continue;
 
 #ifdef SOCKET_DEBUG_TRACE
@@ -2273,7 +2276,7 @@ int HTDoConnect(const char *url,
        pbuf[0] = 0x05;         /* VER: protocol version: X'05' */
        pbuf[1] = 0x01;         /* NMETHODS: 1 */
        pbuf[2] = 0x00;         /* METHOD: X'00' NO AUTHENTICATION REQUIRED */
-       if (write(*s, pbuf, 3) != 3) {
+       if (NETWRITE(*s, pbuf, 3) != 3) {
            goto report_system_err;
        } else if (HTDoRead(*s, pbuf, 2) != 2) {
            goto report_system_err;
@@ -2298,7 +2301,7 @@ int HTDoConnect(const char *url,
            memcpy(&pbuf[i], (unsigned char *) &x, sizeof x);
            i += (unsigned) sizeof(x);
        }
-       if ((size_t) write(*s, pbuf, i) != i) {
+       if ((size_t) NETWRITE(*s, pbuf, i) != i) {
            goto report_system_err;
        } else if ((unsigned) HTDoRead(*s, pbuf, 4) != 4) {
            goto report_system_err;

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)