Skip to content

Commit 8cf2426

Browse files
committed
Fix f-3131 MqttClient_Connect cred clearing tests
1 parent 4f0b145 commit 8cf2426

1 file changed

Lines changed: 98 additions & 0 deletions

File tree

tests/test_mqtt_client.c

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,103 @@ TEST(connect_with_mock_network)
287287
ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc);
288288
}
289289

290+
/* Regression test for tx_buf credential zeroing after CONNECT is sent.
291+
* Guards CLIENT_FORCE_ZERO(client->tx_buf, xfer) in MqttClient_Connect: the
292+
* original issue being prevented is plaintext credentials lingering in the
293+
* client's tx_buf after the CONNECT packet is written. Without this test, a
294+
* regression that deletes that line (or passes length 0) would pass silently. */
295+
#define TEST_CONNECT_USERNAME "user"
296+
#define TEST_CONNECT_PASSWORD "secret"
297+
298+
static int connect_mock_xfer;
299+
static byte connect_mock_sent[TEST_TX_BUF_SIZE];
300+
301+
static int mock_net_write_accept(void *context, const byte* buf, int buf_len,
302+
int timeout_ms)
303+
{
304+
(void)context; (void)timeout_ms;
305+
if (buf != NULL && buf_len > 0 &&
306+
buf_len <= (int)sizeof(connect_mock_sent)) {
307+
XMEMCPY(connect_mock_sent, buf, (size_t)buf_len);
308+
connect_mock_xfer = buf_len;
309+
}
310+
/* Pretend the full packet was sent so MqttClient_Connect reaches the
311+
* CLIENT_FORCE_ZERO step. */
312+
return buf_len;
313+
}
314+
315+
static int buf_contains(const byte* buf, int buf_len,
316+
const char* needle, int needle_len)
317+
{
318+
int i;
319+
if (buf == NULL || needle_len <= 0 || buf_len < needle_len) {
320+
return 0;
321+
}
322+
for (i = 0; i + needle_len <= buf_len; i++) {
323+
if (XMEMCMP(&buf[i], needle, (size_t)needle_len) == 0) {
324+
return 1;
325+
}
326+
}
327+
return 0;
328+
}
329+
330+
TEST(connect_clears_tx_buf_credentials)
331+
{
332+
int rc;
333+
int i;
334+
MqttConnect connect;
335+
const int user_len = (int)sizeof(TEST_CONNECT_USERNAME) - 1;
336+
const int pass_len = (int)sizeof(TEST_CONNECT_PASSWORD) - 1;
337+
338+
rc = test_init_client();
339+
ASSERT_EQ(MQTT_CODE_SUCCESS, rc);
340+
341+
/* Swap in a write mock that accepts the packet and records what was
342+
* sent. Read still returns MQTT_CODE_ERROR_NETWORK so MqttClient_Connect
343+
* returns after the CLIENT_FORCE_ZERO step. */
344+
connect_mock_xfer = 0;
345+
XMEMSET(connect_mock_sent, 0, sizeof(connect_mock_sent));
346+
test_net.write = mock_net_write_accept;
347+
348+
XMEMSET(&connect, 0, sizeof(connect));
349+
connect.keep_alive_sec = 60;
350+
connect.clean_session = 1;
351+
connect.client_id = "test_client";
352+
connect.username = TEST_CONNECT_USERNAME;
353+
connect.password = TEST_CONNECT_PASSWORD;
354+
355+
rc = MqttClient_Connect(&test_client, &connect);
356+
/* The read mock cannot deliver a CONNECT_ACK, so a successful return
357+
* would be wrong regardless of the zeroing step. */
358+
ASSERT_NE(MQTT_CODE_SUCCESS, rc);
359+
360+
/* Confirm the write path actually ran with credentials present. Without
361+
* this, the zeroing assertion below could pass trivially. */
362+
ASSERT_TRUE(connect_mock_xfer > 0);
363+
ASSERT_TRUE(buf_contains(connect_mock_sent, connect_mock_xfer,
364+
TEST_CONNECT_USERNAME, user_len));
365+
ASSERT_TRUE(buf_contains(connect_mock_sent, connect_mock_xfer,
366+
TEST_CONNECT_PASSWORD, pass_len));
367+
368+
/* Core regression check: credentials must not remain in tx_buf after
369+
* MqttClient_Connect returns. Scans the full buffer because the zeroed
370+
* region covers [0..xfer) and the remainder was zero-initialized at
371+
* setup. */
372+
ASSERT_FALSE(buf_contains(test_client.tx_buf, TEST_TX_BUF_SIZE,
373+
TEST_CONNECT_USERNAME, user_len));
374+
ASSERT_FALSE(buf_contains(test_client.tx_buf, TEST_TX_BUF_SIZE,
375+
TEST_CONNECT_PASSWORD, pass_len));
376+
377+
/* Stronger boundary check: every byte the mock observed being written
378+
* must now be zero. This catches both deletion of the CLIENT_FORCE_ZERO
379+
* call and an `xfer` -> `0` mutation that turns the wipe into a no-op. */
380+
for (i = 0; i < connect_mock_xfer; i++) {
381+
if (test_client.tx_buf[i] != 0) {
382+
FAIL("tx_buf byte within xfer range is non-zero after CONNECT");
383+
}
384+
}
385+
}
386+
290387
/* ============================================================================
291388
* MqttClient_Disconnect Tests
292389
* ============================================================================ */
@@ -527,6 +624,7 @@ void run_mqtt_client_tests(void)
527624
RUN_TEST(connect_null_connect);
528625
RUN_TEST(connect_both_null);
529626
RUN_TEST(connect_with_mock_network);
627+
RUN_TEST(connect_clears_tx_buf_credentials);
530628

531629
/* MqttClient_Disconnect tests */
532630
RUN_TEST(disconnect_null_client);

0 commit comments

Comments
 (0)