Avoid aliasing the ticket in wolfSSL_GetSessionAtIndex.

Thanks to Clouditera Security; Z.ai Security; NSFOCUS for the report.

doc/dox_comments/header_files/ssl.h

@@ -2828,7 +2828,10 @@ int wolfSSL_GetSessionIndex(WOLFSSL* ssl);
 
     \brief This function gets the session at specified index of the session
     cache and copies it into memory. The WOLFSSL_SESSION structure holds
-    the session information.
+    the session information. The copy is independent of the cache entry: it
+    does not share the ticket buffer, peer certificate or ex_data with the
+    cache, and it stays valid after the cache entry is overwritten or evicted.
+    The caller owns the copy and releases it with wolfSSL_SESSION_free().
 
     \return SSL_SUCCESS returned if the function executed successfully and
     no errors were thrown.
@@ -2836,16 +2839,19 @@ int wolfSSL_GetSessionIndex(WOLFSSL* ssl);
     \return SSL_FAILURE returned if the function did not execute successfully.
 
     \param index an int type representing the session index.
-    \param session a pointer to the WOLFSSL_SESSION structure.
+    \param session a pointer to a WOLFSSL_SESSION structure to copy into,
+    obtained from wolfSSL_SESSION_new().
 
     _Example_
     \code
     int idx; // The index to locate the session.
-    WOLFSSL_SESSION* session;  // Buffer to copy to.
+    WOLFSSL_SESSION* session = wolfSSL_SESSION_new();  // Buffer to copy to.
     ...
     if(wolfSSL_GetSessionAtIndex(idx, session) != SSL_SUCCESS){
     	// Failure case.
     }
+    ...
+    wolfSSL_SESSION_free(session);
     \endcode
 
     \sa UnLockMutex

src/ssl_sess.c

@@ -2271,8 +2271,18 @@ int wolfSSL_GetSessionAtIndex(int idx, WOLFSSL_SESSION* session)
     cacheSession = &sessRow->Sessions[col];
 #endif
     if (cacheSession) {
-        XMEMCPY(session, cacheSession, sizeof(WOLFSSL_SESSION));
-        result = WOLFSSL_SUCCESS;
+#ifdef HAVE_EX_DATA
+        /* The copy keeps its own ex_data. The struct copy below carries over
+         * the cache's pointers, which the cache frees when the entry goes. */
+        WOLFSSL_CRYPTO_EX_DATA exData;
+        XMEMCPY(&exData, &session->ex_data, sizeof(exData));
+#endif
+        /* Must not alias the ticket, peer cert and ex_data the cache owns and
+         * frees on overwrite or eviction. */
+        result = wolfSSL_DupSession(cacheSession, session, 0);
+#ifdef HAVE_EX_DATA
+        XMEMCPY(&session->ex_data, &exData, sizeof(exData));
+#endif
     }
     else {
         result = WOLFSSL_FAILURE;

tests/api/test_session.c

@@ -1614,3 +1614,128 @@ int test_wolfSSL_SESSION_get_ex_new_index(void)
     return TEST_SKIPPED;
 }
 #endif
+
+/*----------------------------------------------------------------------------*/
+/* wolfSSL_GetSessionAtIndex                                                  */
+/*----------------------------------------------------------------------------*/
+
+#if defined(SESSION_INDEX) && defined(HAVE_SESSION_TICKET) && \
+    !defined(NO_SESSION_CACHE) && !defined(NO_WOLFSSL_CLIENT) && \
+    !defined(NO_TLS)
+
+/* Cache a client session under id with a ticLen byte ticket of fill bytes.
+ * ticLen over SESSION_TICKET_LEN makes the cache allocate a ticket buffer. */
+static int test_session_at_index_add(WOLFSSL_CTX* ctx, const byte* id,
+    word16 ticLen, byte fill, int* idx)
+{
+    EXPECT_DECLS;
+    WOLFSSL_SESSION* sess = NULL;
+    byte* tic = NULL;
+
+    ExpectNotNull(tic = (byte*)XMALLOC(ticLen, NULL, DYNAMIC_TYPE_TMP_BUFFER));
+    ExpectNotNull(sess = wolfSSL_SESSION_new());
+    if (EXPECT_SUCCESS()) {
+        XMEMSET(tic, fill, ticLen);
+        XMEMCPY(sess->sessionID, id, ID_LEN);
+        sess->sessionIDSz = ID_LEN;
+        sess->side = WOLFSSL_CLIENT_END;
+        sess->isSetup = 1;
+        /* Borrowed buffer - ticketLenAlloc stays 0 so sess does not free it. */
+        sess->ticket = tic;
+        sess->ticketLen = ticLen;
+    }
+    ExpectIntEQ(AddSessionToCache(ctx, sess, id, ID_LEN, idx,
+        WOLFSSL_CLIENT_END, 1, NULL), 0);
+
+    if (sess != NULL) {
+        sess->ticket = sess->staticTicket;
+        sess->ticketLen = 0;
+        wolfSSL_SESSION_free(sess);
+    }
+    XFREE(tic, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+
+    return EXPECT_RESULT();
+}
+
+static int test_session_at_index_ticket_is(const WOLFSSL_SESSION* sess,
+    word16 ticLen, byte fill)
+{
+    word16 i;
+
+    if ((sess == NULL) || (sess->ticket == NULL) || (sess->ticketLen != ticLen))
+        return 0;
+    for (i = 0; i < ticLen; i++) {
+        if (sess->ticket[i] != fill)
+            return 0;
+    }
+    return 1;
+}
+
+int test_wolfSSL_GetSessionAtIndex(void)
+{
+    EXPECT_DECLS;
+    WOLFSSL_CTX* ctx = NULL;
+    WOLFSSL_SESSION* copy = NULL;
+    WOLFSSL_SESSION* copy2 = NULL;
+    byte id[ID_LEN];
+    word16 ticLen = (word16)(SESSION_TICKET_LEN + 128);
+    int idx = -1;
+
+    XMEMSET(id, 0x5A, sizeof(id));
+    ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
+    ExpectIntEQ(test_session_at_index_add(ctx, id, ticLen, 0xA1, &idx),
+        TEST_SUCCESS);
+    ExpectIntGE(idx, 0);
+
+    ExpectNotNull(copy = wolfSSL_SESSION_new());
+#ifdef HAVE_EX_DATA
+    if (copy != NULL) {
+        copy->ex_data.ex_data[0] = (void*)copy;
+    }
+#endif
+    ExpectIntEQ(wolfSSL_GetSessionAtIndex(idx, copy), WOLFSSL_SUCCESS);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy, ticLen, 0xA1), 1);
+#ifdef HAVE_EX_DATA
+    /* The copy keeps its own ex_data and the right to free it. */
+    if (copy != NULL) {
+        ExpectPtrEq(copy->ex_data.ex_data[0], (void*)copy);
+        ExpectIntEQ(copy->ownExData, 1);
+    }
+#endif
+
+    /* Each copy owns its ticket rather than pointing into the cache. */
+    ExpectNotNull(copy2 = wolfSSL_SESSION_new());
+    ExpectIntEQ(wolfSSL_GetSessionAtIndex(idx, copy2), WOLFSSL_SUCCESS);
+    if ((copy != NULL) && (copy2 != NULL)) {
+        ExpectPtrNE(copy->ticket, copy2->ticket);
+    }
+
+    /* Same length overwrite reuses the cache buffer - copies keep their
+     * bytes. */
+    ExpectIntEQ(test_session_at_index_add(ctx, id, ticLen, 0xB2, NULL),
+        TEST_SUCCESS);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy, ticLen, 0xA1), 1);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy2, ticLen, 0xA1), 1);
+
+    /* Longer ticket makes the cache free its buffer, and releasing a copy must
+     * not free a buffer the cache still uses. */
+    wolfSSL_SESSION_free(copy2);
+    copy2 = NULL;
+    ExpectIntEQ(test_session_at_index_add(ctx, id, (word16)(ticLen + 512),
+        0xC3, NULL), TEST_SUCCESS);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy, ticLen, 0xA1), 1);
+
+    wolfSSL_SESSION_free(copy);
+    wolfSSL_CTX_free(ctx);
+    return EXPECT_RESULT();
+}
+
+#else
+
+int test_wolfSSL_GetSessionAtIndex(void)
+{
+    return TEST_SKIPPED;
+}
+
+#endif /* SESSION_INDEX && HAVE_SESSION_TICKET && !NO_SESSION_CACHE &&
+        * !NO_WOLFSSL_CLIENT && !NO_TLS */

tests/api/test_session.h

@@ -36,6 +36,7 @@ int test_wolfSSL_SESSION_expire_downgrade(void);
 int test_wolfSSL_CTX_sess_set_remove_cb(void);
 int test_wolfSSL_ticket_keys(void);
 int test_wolfSSL_SESSION_get_ex_new_index(void);
+int test_wolfSSL_GetSessionAtIndex(void);
 
 #define TEST_SESSION_DECLS                                                     \
     TEST_DECL_GROUP("session", test_wolfSSL_CTX_add_session),                  \
@@ -49,6 +50,7 @@ int test_wolfSSL_SESSION_get_ex_new_index(void);
     TEST_DECL_GROUP("session", test_wolfSSL_SESSION_expire_downgrade),         \
     TEST_DECL_GROUP("session", test_wolfSSL_CTX_sess_set_remove_cb),           \
     TEST_DECL_GROUP("session", test_wolfSSL_ticket_keys),                      \
-    TEST_DECL_GROUP("session", test_wolfSSL_SESSION_get_ex_new_index)
+    TEST_DECL_GROUP("session", test_wolfSSL_SESSION_get_ex_new_index),         \
+    TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex)
 
 #endif /* WOLFCRYPT_TEST_SESSION_H */