From 3f7e146dae4c00a1706b9617e8dbf1bd421722b9 Mon Sep 17 00:00:00 2001 From: orbisai0security Date: Thu, 4 Jun 2026 03:24:59 +0000 Subject: [PATCH] fix: add bounds check before memcpy in ssltap.c The memcpy at ssltap --- cmd/ssltap/ssltap.c | 9 ++-- tests/test_invariant_ssltap.c | 96 +++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 tests/test_invariant_ssltap.c diff --git a/cmd/ssltap/ssltap.c b/cmd/ssltap/ssltap.c index 8d300b7922..a427d5b360 100644 --- a/cmd/ssltap/ssltap.c +++ b/cmd/ssltap/ssltap.c @@ -1343,8 +1343,9 @@ print_ssl3_handshake(unsigned char *recordBuf, if (s->msgBufOffset && s->msgBuf) { /* append recordBuf to msgBuf, then use msgBuf */ - if (s->msgBufOffset + recordLen > s->msgBufSize) { - int newSize = s->msgBufOffset + recordLen; + if (s->msgBufOffset > s->msgBufSize - recordLen || + recordLen > s->msgBufSize - s->msgBufOffset) { + unsigned int newSize = s->msgBufOffset + recordLen; unsigned char *newBuf = PORT_Realloc(s->msgBuf, newSize); if (!newBuf) { PR_ASSERT(newBuf); @@ -1588,7 +1589,7 @@ print_ssl3_handshake(unsigned char *recordBuf, "%a, %d-%b-%Y %H:%M:%S GMT", &et); } else { /* 0 means the lifetime of the ticket is unspecified */ - strcpy(lifetime, "unspecified"); + snprintf(lifetime, sizeof(lifetime), "unspecified"); } ticketlength = GET_SHORT((hsdata + 4)); @@ -2203,7 +2204,7 @@ print_hex(int amt, unsigned char *buf) if (i % 16 == 0) { /* if we are at the beginning of a line */ PR_fprintf(PR_STDOUT, "%4x:", i); /* print the line number */ - strcpy(string, ""); + snprintf(string, sizeof(string), ""); } if (i % 4 == 0) { diff --git a/tests/test_invariant_ssltap.c b/tests/test_invariant_ssltap.c new file mode 100644 index 0000000000..aeab841a5f --- /dev/null +++ b/tests/test_invariant_ssltap.c @@ -0,0 +1,96 @@ +#include +#include +#include +#include + +/* Simulate the vulnerable structure and bounds check that MUST hold */ +#define MSG_BUF_SIZE 65536 + +typedef struct { + unsigned char *msgBuf; + unsigned int msgBufSize; + unsigned int msgBufOffset; +} SSLSessionState; + +/* Security invariant: recordLen + msgBufOffset must never exceed msgBufSize */ +static int is_copy_safe(unsigned int msgBufOffset, unsigned int recordLen, unsigned int msgBufSize) +{ + /* Check for overflow and bounds */ + if (recordLen > msgBufSize) + return 0; + if (msgBufOffset > msgBufSize) + return 0; + if (msgBufOffset + recordLen > msgBufSize) + return 0; + return 1; +} + +START_TEST(test_msgbuf_bounds_check) +{ + /* Invariant: memcpy into msgBuf must never write beyond allocated size */ + struct { + unsigned int offset; + unsigned int recordLen; + } payloads[] = { + { 65530, 100 }, /* Exploit: offset + len > bufsize */ + { 0, 65537 }, /* Boundary: recordLen exceeds buffer */ + { 0, 1024 }, /* Valid: normal TLS record */ + { 65535, 1 }, /* Boundary: exactly at limit */ + { 65536, 0 }, /* Edge: offset at max, zero length */ + }; + int num_payloads = sizeof(payloads) / sizeof(payloads[0]); + + SSLSessionState s; + s.msgBufSize = MSG_BUF_SIZE; + s.msgBuf = malloc(s.msgBufSize); + ck_assert_ptr_nonnull(s.msgBuf); + + for (int i = 0; i < num_payloads; i++) { + s.msgBufOffset = payloads[i].offset; + unsigned int recordLen = payloads[i].recordLen; + + int safe = is_copy_safe(s.msgBufOffset, recordLen, s.msgBufSize); + + /* Security property: if bounds check passes, copy must be within bounds */ + if (safe) { + ck_assert_uint_le(s.msgBufOffset + recordLen, s.msgBufSize); + } + /* Adversarial inputs should fail the bounds check */ + if (s.msgBufOffset + recordLen > s.msgBufSize) { + ck_assert_int_eq(safe, 0); + } + } + + free(s.msgBuf); +} +END_TEST + +Suite *security_suite(void) +{ + Suite *s; + TCase *tc_core; + + s = suite_create("Security"); + tc_core = tcase_create("Core"); + + tcase_add_test(tc_core, test_msgbuf_bounds_check); + suite_add_tcase(s, tc_core); + + return s; +} + +int main(void) +{ + int number_failed; + Suite *s; + SRunner *sr; + + s = security_suite(); + sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + number_failed = srunner_ntests_failed(sr); + srunner_free(sr); + + return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} \ No newline at end of file