From 11c67b73e6b0979cd2070c6525beb3458dd4f391 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 4 Aug 2026 11:19:51 +0900 Subject: [PATCH] Append injected data at the end of the input buffer --- doc/dox_comments/header_files-ja/ssl.h | 3 +- doc/dox_comments/header_files/ssl.h | 4 +- src/internal.c | 28 +++-- src/ssl_api_rw.c | 16 ++- tests/api.c | 164 +++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 16 deletions(-) diff --git a/doc/dox_comments/header_files-ja/ssl.h b/doc/dox_comments/header_files-ja/ssl.h index bb88d2aa39a..e905b529a2c 100644 --- a/doc/dox_comments/header_files-ja/ssl.h +++ b/doc/dox_comments/header_files-ja/ssl.h @@ -12950,7 +12950,8 @@ int wolfSSL_read_early_data(WOLFSSL* ssl, void* data, int sz, \param [in] sz 注入するデータのバイト数。 \return BAD_FUNC_ARG いずれかのポインタパラメータがNULLまたはsz <= 0の場合。 - \return APP_DATA_READY 読み取るべきアプリケーションデータが残っている場合。 + \return BUFFER_ERROR 入力バッファの長さに矛盾がある場合。 + \return APP_DATA_READY 読み取るべきアプリケーションデータが残っている状態で入力バッファの拡張が必要になった場合。 \return MEMORY_E 割り当てが失敗した場合。 \return WOLFSSL_SUCCESS 成功時。 diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index 1297fbfe325..6ad24a10fa4 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -15416,7 +15416,9 @@ int wolfSSL_read_early_data(WOLFSSL* ssl, void* data, int sz, \param [in] sz number of bytes of data to inject. \return BAD_FUNC_ARG if any pointer parameter is NULL or sz <= 0 - \return APP_DATA_READY if there is application data left to read + \return BUFFER_ERROR if the input buffer lengths are inconsistent + \return APP_DATA_READY if the input buffer must be grown while there is + application data left to read \return MEMORY_E if allocation fails \return WOLFSSL_SUCCESS on success diff --git a/src/internal.c b/src/internal.c index 3de4b1d060c..7cab756f8ab 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12428,19 +12428,27 @@ int GrowInputBuffer(WOLFSSL* ssl, int size, int usedLength) tmp += align - hdrSz; #endif -#ifdef WOLFSSL_STATIC_MEMORY - /* can be from IO memory pool which does not need copy if same buffer */ - if (usedLength && tmp == ssl->buffers.inputBuffer.buffer) { - ssl->buffers.inputBuffer.bufferSize = size + usedLength; + /* Move the retained data to the front of the new buffer. A pooled + * allocator can hand back the buffer still in use, so the two regions may + * overlap and this has to be a move, not a copy. */ + if (usedLength > 0) { + XMEMMOVE(tmp, ssl->buffers.inputBuffer.buffer + + ssl->buffers.inputBuffer.idx, (size_t)usedLength); + } + + /* Same buffer back: the data is already in place, so it must not be freed + * or zeroed here, only the consumed bytes left behind it. */ + if (tmp == ssl->buffers.inputBuffer.buffer) { + if (IsEncryptionOn(ssl, 1) && + ssl->buffers.inputBuffer.length > (word32)usedLength) { + ForceZero(tmp + usedLength, + ssl->buffers.inputBuffer.length - (word32)usedLength); + } + ssl->buffers.inputBuffer.bufferSize = (word32)(size + usedLength); ssl->buffers.inputBuffer.idx = 0; - ssl->buffers.inputBuffer.length = usedLength; + ssl->buffers.inputBuffer.length = (word32)usedLength; return 0; } -#endif - - if (usedLength) - XMEMCPY(tmp, ssl->buffers.inputBuffer.buffer + - ssl->buffers.inputBuffer.idx, (size_t)(usedLength)); if (ssl->buffers.inputBuffer.dynamicFlag) { if (IsEncryptionOn(ssl, 1)) { diff --git a/src/ssl_api_rw.c b/src/ssl_api_rw.c index e20311f3327..e96becd4b15 100644 --- a/src/ssl_api_rw.c +++ b/src/ssl_api_rw.c @@ -278,6 +278,9 @@ int wolfSSL_write(WOLFSSL* ssl, const void* data, int sz) * @param [in] sz Length of data in bytes. * @return WOLFSSL_SUCCESS on success. * @return BAD_FUNC_ARG when ssl or data is NULL, or sz is not positive. + * @return BUFFER_ERROR when the input buffer lengths are inconsistent. + * @return APP_DATA_READY when application data is waiting to be read and the + * input buffer would have to be grown. * @return MEMORY_E when growing the input buffer fails. */ int wolfSSL_inject(WOLFSSL* ssl, const void* data, int sz) @@ -292,8 +295,12 @@ int wolfSSL_inject(WOLFSSL* ssl, const void* data, int sz) usedLength = (int)(ssl->buffers.inputBuffer.length - ssl->buffers.inputBuffer.idx); + /* free space past everything buffered, where new data is appended */ maxLength = (int)(ssl->buffers.inputBuffer.bufferSize - - (word32)usedLength); + ssl->buffers.inputBuffer.length); + + if (usedLength < 0 || maxLength < 0) + return BUFFER_ERROR; if (sz > maxLength) { /* Need to make space */ @@ -304,14 +311,15 @@ int wolfSSL_inject(WOLFSSL* ssl, const void* data, int sz) WOLFSSL_MSG("Can't inject while there is application data to read"); return APP_DATA_READY; } + /* compacts the unconsumed data, leaving idx 0 and length usedLength */ ret = GrowInputBuffer(ssl, sz, usedLength); if (ret < 0) return ret; } - XMEMCPY(ssl->buffers.inputBuffer.buffer + ssl->buffers.inputBuffer.idx, - data, sz); - ssl->buffers.inputBuffer.length += sz; + XMEMCPY(ssl->buffers.inputBuffer.buffer + ssl->buffers.inputBuffer.length, + data, (size_t)sz); + ssl->buffers.inputBuffer.length += (word32)sz; return WOLFSSL_SUCCESS; } diff --git a/tests/api.c b/tests/api.c index 4e36c73dec2..7b1cfba0ec9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -37422,6 +37422,169 @@ static int test_wolfSSL_inject(void) return EXPECT_RESULT(); } +/* Kept under one TLS record: injecting several records at once is a separate + * (unrelated) code path. */ +#define TEST_INJECT_BIG_SZ 8192 + +/* Injected bytes must land after the data buffered but not yet consumed. + * Writing them at inputBuffer.idx overwrote a retained partial record, so a + * record fed in over several calls never reassembled. */ +static int test_wolfSSL_inject_partial_record(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(NO_SHA256) && \ + !defined(WOLFSSL_NO_TLS12) + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char msg[] = "wolfSSL inject partial record"; + byte* bigData = NULL; + byte* record = NULL; + char* readBuf = NULL; + word32 savedIdx = 0; + word32 savedLength = 0; + int msgSz = (int)sizeof(msg) - 1; + int recordSz = 0; + int rounds = 0; + int injectSz = 0; + int off; + int chunk; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + /* sized from the transport buffer the records are copied out of */ + record = (byte*)XMALLOC(TEST_MEMIO_BUF_SZ, NULL, DYNAMIC_TYPE_TMP_BUFFER); + bigData = (byte*)XMALLOC(TEST_INJECT_BIG_SZ, NULL, DYNAMIC_TYPE_TMP_BUFFER); + readBuf = (char*)XMALLOC(TEST_INJECT_BIG_SZ, NULL, DYNAMIC_TYPE_TMP_BUFFER); + ExpectNotNull(record); + ExpectNotNull(bigData); + ExpectNotNull(readBuf); + /* non-repeating, so the readback also catches reordered chunks */ + if (bigData != NULL) { + for (off = 0; off < TEST_INJECT_BIG_SZ; off++) + bigData[off] = (byte)(off ^ (off >> 8)); + } + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, &rounds), 0); + + /* Draining the transport leaves injected bytes as the server's only input. */ + ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgSz), msgSz); + ExpectIntGT(test_ctx.s_len, 0); + if (EXPECT_SUCCESS()) { + recordSz = test_ctx.s_len; + XMEMCPY(record, test_ctx.s_buff, (size_t)recordSz); + test_memio_clear_buffer(&test_ctx, 0); + } + + /* Every chunk but the last leaves a partial record buffered. */ + for (off = 0; off < recordSz && EXPECT_SUCCESS(); off += chunk) { + chunk = recordSz - off; + if (chunk > 7) + chunk = 7; + ExpectIntEQ(wolfSSL_inject(ssl_s, record + off, chunk), + WOLFSSL_SUCCESS); + if (off + chunk < recordSz) { + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, msgSz), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + } + } + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, msgSz), msgSz); + ExpectIntEQ(XMEMCMP(readBuf, msg, (size_t)msgSz), 0); + + /* A first chunk shorter than a record header leaves nothing to pre-grow the + * buffer, so the second inject grows it with a partial record held. */ + ExpectIntEQ(wolfSSL_write(ssl_c, bigData, TEST_INJECT_BIG_SZ), + TEST_INJECT_BIG_SZ); + ExpectIntGT(test_ctx.s_len, TEST_INJECT_BIG_SZ); + if (EXPECT_SUCCESS()) { + recordSz = test_ctx.s_len; + XMEMCPY(record, test_ctx.s_buff, (size_t)recordSz); + test_memio_clear_buffer(&test_ctx, 0); + } + + ExpectIntEQ(wolfSSL_inject(ssl_s, record, 3), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, TEST_INJECT_BIG_SZ), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(wolfSSL_inject(ssl_s, record + 3, recordSz - 3), + WOLFSSL_SUCCESS); + /* Configurations whose static buffer already holds a whole record cover the + * no-grow path here instead. */ + if (ssl_s != NULL && STATIC_BUFFER_LEN < TEST_INJECT_BIG_SZ) { + ExpectIntGT(ssl_s->buffers.inputBuffer.bufferSize, + (word32)STATIC_BUFFER_LEN); + } + + /* The payload may arrive as several records, so read until it is drained. */ + for (off = 0; off < TEST_INJECT_BIG_SZ && EXPECT_SUCCESS(); off += chunk) { + chunk = wolfSSL_read(ssl_s, readBuf, TEST_INJECT_BIG_SZ - off); + ExpectIntGT(chunk, 0); + if (chunk <= 0) + break; + ExpectIntEQ(XMEMCMP(readBuf, bigData + off, (size_t)chunk), 0); + } + ExpectIntEQ(off, TEST_INJECT_BIG_SZ); + + /* Reading a single byte leaves the rest of the plaintext pending. */ + ExpectIntEQ(wolfSSL_write(ssl_c, msg, msgSz), msgSz); + ExpectIntGT(test_ctx.s_len, 0); + if (EXPECT_SUCCESS()) { + recordSz = test_ctx.s_len; + XMEMCPY(record, test_ctx.s_buff, (size_t)recordSz); + test_memio_clear_buffer(&test_ctx, 0); + } + ExpectIntEQ(wolfSSL_inject(ssl_s, record, recordSz), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, 1), 1); + ExpectIntEQ(wolfSSL_pending(ssl_s), msgSz - 1); + + /* clearOutputBuffer points into the input buffer, so a grow has to be + * refused instead of invalidating that pending plaintext. Size the inject + * past the free space so it needs a grow whatever the buffer size is. */ + if (ssl_s != NULL) { + injectSz = (int)(ssl_s->buffers.inputBuffer.bufferSize - + ssl_s->buffers.inputBuffer.length) + 1; + } + ExpectIntGT(injectSz, 0); + ExpectIntLE(injectSz, TEST_MEMIO_BUF_SZ); + ExpectIntEQ(wolfSSL_inject(ssl_s, record, injectSz), + WC_NO_ERR_TRACE(APP_DATA_READY)); + ExpectIntEQ(wolfSSL_read(ssl_s, readBuf, msgSz - 1), msgSz - 1); + ExpectIntEQ(XMEMCMP(readBuf, msg + 1, (size_t)(msgSz - 1)), 0); + + /* A broken buffer invariant must fail closed: both lengths are unsigned + * differences, so idx > length or length > bufferSize goes negative. The + * call returns before it writes, so restoring the field is enough. */ + if (ssl_s != NULL) { + savedIdx = ssl_s->buffers.inputBuffer.idx; + ssl_s->buffers.inputBuffer.idx = ssl_s->buffers.inputBuffer.length + 1; + } + ExpectIntEQ(wolfSSL_inject(ssl_s, record, 1), + WC_NO_ERR_TRACE(BUFFER_ERROR)); + if (ssl_s != NULL) { + ssl_s->buffers.inputBuffer.idx = savedIdx; + savedLength = ssl_s->buffers.inputBuffer.length; + ssl_s->buffers.inputBuffer.length = + ssl_s->buffers.inputBuffer.bufferSize + 1; + } + ExpectIntEQ(wolfSSL_inject(ssl_s, record, 1), + WC_NO_ERR_TRACE(BUFFER_ERROR)); + if (ssl_s != NULL) + ssl_s->buffers.inputBuffer.length = savedLength; + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + XFREE(record, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(bigData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(readBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} + /*----------------------------------------------------------------------------* | Main *----------------------------------------------------------------------------*/ @@ -39396,6 +39559,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_read_ahead_buffer_len), TEST_DECL(test_wolfSSL_read_ahead_ctx_inherit), TEST_DECL(test_wolfSSL_inject), + TEST_DECL(test_wolfSSL_inject_partial_record), TEST_DECL(test_ocsp_status_callback), TEST_DECL(test_ocsp_basic_verify), TEST_DECL(test_ocsp_ancestor_responder_rejected),