Fix from review

Author: embhorn

src/tls.c

@@ -14770,8 +14770,18 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
      * pushes the running total past 0xFFFF is detected rather than
      * silently wrapped (the TLS extensions block length prefix on the
      * wire is a 2-byte field). Callees that take a word16* accumulator
-     * are invoked via a per-iteration shim and their delta is added
-     * back into the word32 total. */
+     * are invoked via a per-iteration shim (`cbShim`) and their delta
+     * is added back into the word32 total.
+     *
+     * MAINTAINER NOTE: do NOT pass &length to any *_GET_SIZE function
+     * that expects a `word16*` out-parameter — that would be a type
+     * mismatch (UB) and would silently bypass the overflow detection
+     * below. When adding a new extension case, either:
+     *   - use `length += FOO_GET_SIZE(...)` when the helper returns a
+     *     word16 by value, or
+     *   - use the cbShim pattern: `cbShim = 0; ret = FOO_GET_SIZE(...,
+     *     &cbShim); length += cbShim;`
+     */
     word32 length = 0;
     word16 cbShim;
     byte   isRequest = (msgType == client_hello ||
@@ -14964,16 +14974,18 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType,
                 break;
         }
 
-        /* marks the extension as processed so ctx level */
-        /* extensions don't overlap with ssl level ones. */
-        TURN_ON(semaphore, TLSX_ToSemaphore((word16)extension->type));
-
         /* Early exit: stop accumulating as soon as the running total
-         * cannot possibly fit the 2-byte wire length. */
+         * cannot possibly fit the 2-byte wire length. Check *before*
+         * marking the extension as processed so the semaphore is not
+         * left in an inconsistent state on the error path. */
         if (length > WOLFSSL_MAX_16BIT) {
             WOLFSSL_MSG("TLSX_GetSize extension length exceeds word16");
             return BUFFER_E;
         }
+
+        /* marks the extension as processed so ctx level */
+        /* extensions don't overlap with ssl level ones. */
+        TURN_ON(semaphore, TLSX_ToSemaphore((word16)extension->type));
     }
 
     if ((word32)*pLength + length > WOLFSSL_MAX_16BIT) {

tests/api.c

@@ -10528,6 +10528,83 @@ static int test_tls_ext_word16_overflow(void)
     wolfSSL_free(ssl);
     wolfSSL_CTX_free(ctx);
     XFREE(big, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    ssl = NULL;
+    ctx = NULL;
+    big = NULL;
+
+    /* Boundary case: construct a SessionTicket extension sized so that the
+     * total extensions length in TLSX_GetRequestSize is exactly
+     * WOLFSSL_MAX_16BIT - OPAQUE16_LEN (0xFFFD) *before* the OPAQUE16_LEN
+     * block-prefix adjustment, which must succeed. This pins the `>`
+     * comparison in the overflow check — mutating it to `>=` would
+     * incorrectly reject this valid case and fail this test. */
+    {
+        WOLFSSL_CTX* ctx2 = NULL;
+        WOLFSSL*     ssl2 = NULL;
+        SessionTicket* ticket2 = NULL;
+        byte* buf = NULL;
+        word32 baseLen = 0;
+        word32 baseInternal = 0;
+        word32 tickSz = 0;
+        /* TLSX_GetRequestSize rejects when internal sum > 0xFFFD. */
+        const word32 target = (word32)WOLFSSL_MAX_16BIT - (word32)OPAQUE16_LEN;
+        /* Session ticket extension contributes: type (2) + len (2) + size. */
+        const word32 extHdr = (word32)HELLO_EXT_TYPE_SZ
+                            + (word32)OPAQUE16_LEN;
+        int ret;
+
+        ExpectNotNull(ctx2 = wolfSSL_CTX_new(wolfSSLv23_client_method()));
+        ExpectNotNull(ssl2 = wolfSSL_new(ctx2));
+
+        /* Measure baseline length with no session ticket extension. The
+         * returned value already includes the 2-byte block-length prefix
+         * when nonzero; strip it to get the raw internal sum. */
+        if (EXPECT_SUCCESS()) {
+            baseLen = 0;
+            ret = TLSX_GetRequestSize(ssl2, client_hello, &baseLen);
+            ExpectIntEQ(ret, 0);
+            baseInternal = (baseLen > 0)
+                           ? baseLen - (word32)OPAQUE16_LEN : 0;
+        }
+
+        /* Target: baseInternal + extHdr + tickSz == 0xFFFD. */
+        if (EXPECT_SUCCESS() && baseInternal + extHdr < target) {
+            tickSz = target - baseInternal - extHdr;
+        }
+
+        if (EXPECT_SUCCESS() && tickSz > 0 && tickSz <= WOLFSSL_MAX_16BIT) {
+            buf = (byte*)XMALLOC(tickSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+            ExpectNotNull(buf);
+            if (buf != NULL)
+                XMEMSET(buf, 0x5A, tickSz);
+        }
+
+        if (EXPECT_SUCCESS() && buf != NULL) {
+            ticket2 = TLSX_SessionTicket_Create(0, buf, (word16)tickSz,
+                                                ssl2->heap);
+            ExpectNotNull(ticket2);
+        }
+        if (EXPECT_SUCCESS() && ticket2 != NULL) {
+            ExpectIntEQ(TLSX_UseSessionTicket(&ssl2->extensions, ticket2,
+                                              ssl2->heap), WOLFSSL_SUCCESS);
+            ticket2 = NULL;
+        }
+
+        /* Exact boundary: internal sum == 0xFFFD must succeed, and the
+         * final returned length is 0xFFFD + OPAQUE16_LEN == 0xFFFF. */
+        if (EXPECT_SUCCESS()) {
+            word32 lenBoundary = 0;
+            ret = TLSX_GetRequestSize(ssl2, client_hello, &lenBoundary);
+            ExpectIntEQ(ret, 0);
+            ExpectIntEQ(lenBoundary, (word32)WOLFSSL_MAX_16BIT);
+        }
+
+        if (ticket2 != NULL)
+            TLSX_SessionTicket_Free(ticket2, ssl2->heap);
+        wolfSSL_free(ssl2);
+        wolfSSL_CTX_free(ctx2);
+        XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    }
 #endif
     return EXPECT_RESULT();
 }