Fenrir fixes Sep 4 26

src/client/client.c

@@ -35,6 +35,7 @@
 #include <wolfssl/wolfcrypt/settings.h>
 
 #include <ctype.h>
+#include <limits.h>
 
 #include <wolfssl/ssl.h>
 
@@ -95,6 +96,11 @@ static const char *wolfsentry_config_path = NULL;
 #define MAX_NON_BLOCK_SEC   10
 #endif
 
+/* Scratch buffer for a decimal command line argument. LONG_MAX is 19 decimal
+ * digits, so 24 leaves room for that plus a NUL and a little slack; anything
+ * longer is out of range for the values parsed with it. */
+#define MAX_DECIMAL_ARG_LEN 24
+
 #define OCSP_STAPLING 1
 #define OCSP_STAPLINGV2 2
 #define OCSP_STAPLINGV2_MULTI 3
@@ -236,6 +242,7 @@ static WC_INLINE void clu_build_addr(SOCKADDR_IN4_T* addr, SOCKADDR_IN6_T* ipv6,
             if (getaddrinfo((char*)peer, portStr, &hints, &addrInfo) == 0) {
                 XMEMCPY(addr, addrInfo->ai_addr, sizeof(*addr));
                 useLookup = 1;
+                freeaddrinfo(addrInfo);
             }
         #endif
             else
@@ -311,22 +318,20 @@ static WC_INLINE void clu_build_addr(SOCKADDR_IN4_T* addr, SOCKADDR_IN6_T* ipv6,
 static WC_INLINE void clu_tcp_socket(SOCKET_T* sockfd, int udp, int sctp,
         int isIpv6)
 {
+    /* The address built by clu_build_addr() is AF_INET6 whenever isIpv6 is
+     * set, so every socket type has to be opened in the matching family. */
+    int family = isIpv6 ? AF_INET6_V : AF_INET_V;
+
     (void)sctp;
 
     if (udp)
-        *sockfd = socket(AF_INET_V, SOCK_DGRAM, IPPROTO_UDP);
+        *sockfd = socket(family, SOCK_DGRAM, IPPROTO_UDP);
 #ifdef WOLFSSL_SCTP
     else if (sctp)
-        *sockfd = socket(AF_INET_V, SOCK_STREAM, IPPROTO_SCTP);
+        *sockfd = socket(family, SOCK_STREAM, IPPROTO_SCTP);
 #endif
-    else {
-        if (isIpv6) {
-            *sockfd = socket(AF_INET6_V, SOCK_STREAM, IPPROTO_TCP);
-        }
-        else {
-            *sockfd = socket(AF_INET_V, SOCK_STREAM, IPPROTO_TCP);
-        }
-    }
+    else
+        *sockfd = socket(family, SOCK_STREAM, IPPROTO_TCP);
     if(WOLFSSL_SOCKET_IS_INVALID(*sockfd)) {
         err_sys_with_errno("socket failed\n");
     }
@@ -371,7 +376,11 @@ static WC_INLINE void clu_tcp_connect(SOCKET_T* sockfd, const char* ip,
 
     if (isIpv6) {
         clu_build_addr(NULL, &ipv6, ip, port, udp, sctp);
+        if (udp) {
+            wolfSSL_dtls_set_peer(ssl, &ipv6, sizeof(ipv6));
+        }
         clu_tcp_socket(sockfd, udp, sctp, isIpv6);
+
         if (!udp) {
             if (WOLFSSL_SOCKET_IS_INVALID(*sockfd))
                 err_sys_with_errno("tcp bad socket");
@@ -1103,6 +1112,7 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port,
     if (ret == WOLFSSL_SUCCESS) {
         /* Perform throughput test */
         char *tx_buffer, *rx_buffer;
+        const char* errMsg = NULL;
 
         /* Record connection time */
         conn_time = current_time(0) - start;
@@ -1133,9 +1143,13 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port,
                 xfer_bytes = 0;
                 while (throughput > xfer_bytes) {
                     int len, rx_pos, select_ret;
+                    size_t remain;
 
-                    /* Determine packet size */
-                    len = min(block, (int)(throughput - xfer_bytes));
+                    /* Determine packet size. Bound the size_t remainder by
+                     * the block size before narrowing it to int, otherwise a
+                     * remainder above INT_MAX becomes a negative length. */
+                    remain = throughput - xfer_bytes;
+                    len = (remain > (size_t)block) ? block : (int)remain;
 
                     /* Perform TX */
                     start = current_time(1);
@@ -1154,48 +1168,64 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port,
                     } while (err == WC_PENDING_E);
                     if (ret != len) {
                         printf("SSL_write bench error %d!\n", err);
-                        if (!exitWithRet)
-                            err_sys("SSL_write failed");
+                        if (err == 0)
+                            err = WOLFSSL_FATAL_ERROR;
+                        errMsg = "SSL_write failed";
                         goto doExit;
                     }
                     tx_time += current_time(0) - start;
 
                     /* Perform RX */
                     select_ret = tcp_select(sockfd, DEFAULT_TIMEOUT_SEC);
-                    if (select_ret == TEST_RECV_READY) {
-                        start = current_time(1);
-                        rx_pos = 0;
-                        while (rx_pos < len) {
-                            ret = wolfSSL_read(ssl, &rx_buffer[rx_pos],
-                                                                len - rx_pos);
-                            if (ret <= 0) {
-                                err = wolfSSL_get_error(ssl, 0);
-                            #ifdef WOLFSSL_ASYNC_CRYPT
-                                if (err == WC_PENDING_E) {
-                                    ret = wolfSSL_AsyncPoll(ssl, WOLF_POLL_FLAG_CHECK_HW);
-                                    if (ret < 0) break;
-                                }
-                                else
-                            #endif
-                                if (err != WOLFSSL_ERROR_WANT_READ) {
-                                    printf("SSL_read bench error %d\n", err);
-                                    err_sys("SSL_read failed");
-                                }
+                    if (select_ret != TEST_RECV_READY) {
+                        printf("SSL_read bench select error %d!\n", select_ret);
+                        err = WOLFSSL_FATAL_ERROR;
+                        errMsg = (select_ret == TEST_TIMEOUT) ?
+                                "SSL_read timeout" : "SSL_read select failed";
+                        goto doExit;
+                    }
+
+                    start = current_time(1);
+                    rx_pos = 0;
+                    while (rx_pos < len) {
+                        ret = wolfSSL_read(ssl, &rx_buffer[rx_pos],
+                                                            len - rx_pos);
+                        if (ret <= 0) {
+                            err = wolfSSL_get_error(ssl, 0);
+                        #ifdef WOLFSSL_ASYNC_CRYPT
+                            if (err == WC_PENDING_E) {
+                                ret = wolfSSL_AsyncPoll(ssl, WOLF_POLL_FLAG_CHECK_HW);
+                                if (ret < 0) break;
                             }
-                            else {
-                                rx_pos += ret;
+                            else
+                        #endif
+                            if (err != WOLFSSL_ERROR_WANT_READ) {
+                                break;
                             }
                         }
-                        rx_time += current_time(0) - start;
+                        else {
+                            rx_pos += ret;
+                        }
+                    }
+                    rx_time += current_time(0) - start;
+
+                    /* Only compare once the full block has been received */
+                    if (rx_pos != len) {
+                        printf("SSL_read bench error %d!\n", err);
+                        /* wolfSSL_read() can return <= 0 without setting an
+                         * error code, so normalize it here. Otherwise a short
+                         * read would be returned as EXIT_SUCCESS under -H. */
+                        if (err == 0)
+                            err = WOLFSSL_FATAL_ERROR;
+                        errMsg = "SSL_read failed";
+                        goto doExit;
                     }
 
                     /* Compare TX and RX buffers */
                     if (XMEMCMP(tx_buffer, rx_buffer, len) != 0) {
-                        free(tx_buffer);
-                        tx_buffer = NULL;
-                        free(rx_buffer);
-                        rx_buffer = NULL;
-                        err_sys("Compare TX and RX buffers failed");
+                        err = WOLFSSL_FATAL_ERROR;
+                        errMsg = "Compare TX and RX buffers failed";
+                        goto doExit;
                     }
 
                     /* Update overall position */
@@ -1213,6 +1243,10 @@ static int ClientBenchmarkThroughput(WOLFSSL_CTX* ctx, char* host, word16 port,
 doExit:
         if (tx_buffer) XFREE(tx_buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER);
         if (rx_buffer) XFREE(rx_buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        /* err_sys() does not return, so only report the failure once the
+         * buffers have been released */
+        if (errMsg != NULL && !exitWithRet)
+            err_sys(errMsg);
     }
     else {
         err_sys("wolfSSL_connect failed");
@@ -2486,12 +2520,23 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
                 break;
 
             case 'p' :
-                port = (word16)atoi(myoptarg);
+            {
+                long  portArg = 0;
+                /* 0 is not a usable port to connect to, and the port == 0
+                 * check below is compiled out of every wolfCLU build
+                 * (wolfclu/client.h defines NO_MAIN_DRIVER), so reject it
+                 * here. */
+                if (wolfCLU_parseDecimalBounded(myoptarg, 1, 65535, &portArg)
+                        != WOLFCLU_SUCCESS) {
+                    err_sys("port number must be between 1 and 65535");
+                }
+                port = (word16)portArg;
                 #if !defined(NO_MAIN_DRIVER) || defined(USE_WINDOWS_API)
                     if (port == 0)
                         err_sys("port number cannot be 0");
                 #endif
                 break;
+            }
 
             case 'v' :
                 if (myoptarg[0] == 'd') {
@@ -2598,18 +2643,42 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args)
                 break;
 
             case 'B' :
-                throughput = atol(myoptarg);
-                for (; *myoptarg != '\0'; myoptarg++) {
-                    if (*myoptarg == ',') {
-                        block = atoi(myoptarg + 1);
-                        break;
-                    }
+            {
+                long   throughputArg = 0;
+                long   blockArg      = (long)block;
+                const char* comma    = XSTRSTR(myoptarg, ",");
+                size_t numLen        = (comma != NULL) ?
+                        (size_t)(comma - myoptarg) : XSTRLEN(myoptarg);
+                char   numBuf[MAX_DECIMAL_ARG_LEN];
+
+                /* Parse both halves with the bounded decimal parser so that
+                 * negative, out-of-range and trailing-junk input is rejected
+                 * instead of silently saturating or truncating the way
+                 * atol()/atoi() do, which would let the benchmark run with a
+                 * wrapped size_t throughput. */
+                if (numLen == 0 || numLen >= sizeof(numBuf)) {
+                    Usage();
+                    XEXIT_T(MY_EX_USAGE);
                 }
-                if (throughput == 0 || block <= 0) {
+                XMEMCPY(numBuf, myoptarg, numLen);
+                numBuf[numLen] = '\0';
+
+                if (wolfCLU_parseDecimalBounded(numBuf, 1, LONG_MAX,
+                            &throughputArg) != WOLFCLU_SUCCESS) {
+                    Usage();
+                    XEXIT_T(MY_EX_USAGE);
+                }
+                if (comma != NULL &&
+                        wolfCLU_parseDecimalBounded(comma + 1, 1, INT_MAX,
+                            &blockArg) != WOLFCLU_SUCCESS) {
                     Usage();
                     XEXIT_T(MY_EX_USAGE);
                 }
+
+                throughput = (size_t)throughputArg;
+                block      = (int)blockArg;
                 break;
+            }
 
             case 'N' :
                 nonBlocking = 1;

tests/client/client-test.py

@@ -7,7 +7,8 @@
 import unittest
 
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
-from wolfclu_test import WOLFSSL_BIN, CERTS_DIR, run_wolfssl, test_main
+from wolfclu_test import (WOLFSSL_BIN, CERTS_DIR, run_wolfssl,
+                          skip_if_no_filesystem, test_main)
 
 
 class ClientTest(unittest.TestCase):
@@ -17,11 +18,7 @@ def setUpClass(cls):
         if not os.path.isdir(CERTS_DIR):
             raise unittest.SkipTest("certs directory not found")
 
-        config_log = os.path.join(".", "config.log")
-        if os.path.isfile(config_log):
-            with open(config_log, "r") as f:
-                if "disable-filesystem" in f.read():
-                    raise unittest.SkipTest("filesystem support disabled")
+        skip_if_no_filesystem()
 
     def test_s_client_x509(self):
         """Connect to a TLS server, extract cert, and verify PEM output."""
@@ -66,6 +63,65 @@ def test_client_help(self):
         self.assertEqual(r.returncode, 0, r.stderr)
         self.assertIn("s_client" , r.stderr, "help menu was not printed")
 
+class PortArgTest(unittest.TestCase):
+    """Regression tests for the port half of -connect <host>:<port>.
+
+    -connect forwards the text after ':' to the client's -p handler, which
+    used to be (word16)atoi(): "99999" silently truncated to 34463 and the
+    client happily connected to the wrong port.  The port is now parsed with
+    wolfCLU_parseDecimalBounded(), so anything outside 1-65535, and anything
+    that is not a plain decimal number, is rejected before any connection is
+    attempted.
+    """
+
+    PORT_ERROR = "port number must be between 1 and 65535"
+
+    @classmethod
+    def setUpClass(cls):
+        # s_client is compiled out under --disable-filesystem, so it never
+        # reaches the port parser and never prints PORT_ERROR.
+        skip_if_no_filesystem()
+
+    def _assert_rejected(self, port, description):
+        """Run s_client against localhost:<port> and require that the port
+        itself was rejected -- a non-zero exit alone is not enough, since a
+        truncated port would also fail to connect."""
+        r = run_wolfssl("s_client", "-connect", "localhost:" + port,
+                        timeout=30)
+        self.assertNotEqual(r.returncode, 0,
+                            f"{description} port {port!r} was accepted")
+        self.assertIn(self.PORT_ERROR, r.stdout + r.stderr,
+                      f"{description} port {port!r} was not rejected as an "
+                      f"out-of-range port: {r.stdout + r.stderr}")
+
+    def test_port_zero(self):
+        """0 must be rejected: it is not a port that can be connected to."""
+        self._assert_rejected("0", "zero")
+
+    def test_port_above_word16(self):
+        """99999 must be rejected, not truncated to 34463."""
+        self._assert_rejected("99999", "out-of-range")
+
+    def test_port_just_above_word16(self):
+        """65536 must be rejected, not truncated to 0."""
+        self._assert_rejected("65536", "out-of-range")
+
+    def test_port_negative(self):
+        """A negative port must be rejected, not wrapped."""
+        self._assert_rejected("-1", "negative")
+
+    def test_port_non_numeric(self):
+        """A non-numeric port must be rejected, not read as 0."""
+        self._assert_rejected("abc", "non-numeric")
+
+    def test_port_trailing_junk(self):
+        """Trailing junk must be rejected, not silently ignored."""
+        self._assert_rejected("443abc", "trailing junk")
+
+    def test_port_empty(self):
+        """An empty port must be rejected."""
+        self._assert_rejected("", "empty")
+
 class ShellInjectionTest(unittest.TestCase):
     """Regression tests for shell command injection via hostname.
 

tests/wolfclu_test.py

@@ -88,6 +88,20 @@ def run_wolfssl(*args, stdin_data=None, timeout=60):
     return subprocess.run(cmd, **kwargs)
 
 
+def skip_if_no_filesystem():
+    """Raise SkipTest when wolfCLU was built with --disable-filesystem.
+
+    WOLFCLU_NO_FILESYSTEM compiles out the front ends that need files (e.g.
+    wolfCLU_Client), so those commands only print "No filesystem support"
+    instead of doing any work.
+    """
+    config_log = os.path.join(".", "config.log")
+    if os.path.isfile(config_log):
+        with open(config_log, "r") as f:
+            if "disable-filesystem" in f.read():
+                raise unittest.SkipTest("filesystem support disabled")
+
+
 def is_fips():
     """True when linked against a FIPS wolfSSL build (per `wolfssl -v`)."""
     r = run_wolfssl("-v")