diff --git a/src/bio.c b/src/bio.c index 13fdef3a5d..b9dfe6b7dd 100644 --- a/src/bio.c +++ b/src/bio.c @@ -120,6 +120,16 @@ static int wolfSSL_BIO_MEMORY_read(WOLFSSL_BIO* bio, void* buf, int len) WOLFSSL_ENTER("wolfSSL_BIO_MEMORY_read"); } + /* Reject a negative length up front. Callers are expected to validate, but + * guarding here too prevents a negative len from defeating the signed + * bounds checks below (sz/memSz comparisons) and reaching XMEMCPY with a + * length of (size_t)-1. Return the same error the public wolfSSL_BIO_read() + * does for a negative length rather than silently reporting 0 bytes read. A + * zero length is handled correctly by the logic below (copies nothing). */ + if (len < 0) { + return WOLFSSL_BIO_ERROR; + } + sz = wolfSSL_BIO_pending(bio); if (sz > 0) { int memSz; diff --git a/tests/api/test_ossl_bio.c b/tests/api/test_ossl_bio.c index 45c8e94086..b111fd4468 100644 --- a/tests/api/test_ossl_bio.c +++ b/tests/api/test_ossl_bio.c @@ -980,6 +980,45 @@ int test_wolfSSL_BIO_write(void) return EXPECT_RESULT(); } +/* A negative length must never defeat the memory-BIO read bounds check and + * reach XMEMCPY with (size_t)-1. This exercises the public BIO_read() boundary + * (which rejects a negative length before dispatch); the matching guard in the + * static wolfSSL_BIO_MEMORY_read() sink is defense-in-depth and not separately + * reachable through the public API. Verify a negative length is rejected with + * an error without copying, a zero length reads nothing, and the pending data + * is left intact and still readable. */ +int test_wolfSSL_BIO_read_negative_len(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) + BIO* bio = NULL; + char msg[] = "negative length test"; + int msgLen = (int)XSTRLEN(msg); + char out[64]; + + ExpectNotNull(bio = BIO_new(BIO_s_mem())); + ExpectIntEQ(BIO_write(bio, msg, msgLen), msgLen); + + /* Negative length: must be rejected with an error, not a wild copy and not + * a silent 0-byte read. */ + XMEMSET(out, 0, sizeof(out)); + ExpectIntLT(BIO_read(bio, out, -1), 0); + /* Data must be untouched - still all pending. */ + ExpectIntEQ(BIO_pending(bio), msgLen); + + /* Zero length: nothing read, data still pending. */ + ExpectIntEQ(BIO_read(bio, out, 0), 0); + ExpectIntEQ(BIO_pending(bio), msgLen); + + /* A normal read still returns the intact message. */ + ExpectIntEQ(BIO_read(bio, out, (int)sizeof(out)), msgLen); + ExpectIntEQ(XMEMCMP(out, msg, msgLen), 0); + + BIO_free(bio); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_BIO_printf(void) { diff --git a/tests/api/test_ossl_bio.h b/tests/api/test_ossl_bio.h index d401193b14..acf13fb776 100644 --- a/tests/api/test_ossl_bio.h +++ b/tests/api/test_ossl_bio.h @@ -35,6 +35,7 @@ int test_wolfSSL_BIO_datagram(void); int test_wolfSSL_BIO_s_null(void); int test_wolfSSL_BIO_accept(void); int test_wolfSSL_BIO_write(void); +int test_wolfSSL_BIO_read_negative_len(void); int test_wolfSSL_BIO_printf(void); int test_wolfSSL_BIO_f_md(void); int test_wolfSSL_BIO_up_ref(void); @@ -55,6 +56,7 @@ int test_wolfSSL_BIO_get_init(void); TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_should_retry), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_s_null), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_write), \ + TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_read_negative_len), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_printf), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_f_md), \ TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_up_ref), \