security: harden untrusted-input handling (issue #148)
All checks were successful
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Check coding style (pull_request) Successful in 26s
CI Code / Code Coverage (pull_request) Successful in 3m29s
CI Code / Linux (debian) (pull_request) Successful in 5m13s
CI Code / Linux (ubuntu) (pull_request) Successful in 5m16s
CI Code / Linux (arch) (pull_request) Successful in 7m34s
All checks were successful
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Check coding style (pull_request) Successful in 26s
CI Code / Code Coverage (pull_request) Successful in 3m29s
CI Code / Linux (debian) (pull_request) Successful in 5m13s
CI Code / Linux (ubuntu) (pull_request) Successful in 5m16s
CI Code / Linux (arch) (pull_request) Successful in 7m34s
T02: guard the receive-path handlers that dereferenced jid_create() without a NULL check — MUC join errors, subscribed/unsubscribed presence and, with silence.non-roster enabled, every incoming message. A stanza with a missing or malformed 'from' crashed the client (REQ-INP-01) T11: restrict /url open and /url save to http, https and aesgcm, so a received file:, javascript: or data: URL is refused (REQ-INP-06); spawn terminal-notifier through g_spawn_async with an argv instead of building a shell command for system() (REQ-INP-07); apply the XEP-0359 disco gate to MAM result ids, as live stanza-ids already do (REQ-INP-05); replace control and bidi-reordering characters in incoming message bodies with U+FFFD before they reach the terminal, the logs and the database, keeping LRM/RLM for legitimate RTL text (REQ-INP-08); cover JID part-length boundaries and invalid UTF-8 (REQ-INP-02) T10: replace strcpy/strcat/alloca and sprintf with g_strdup_printf and g_snprintf (REQ-MEM-03); allocate the OMEMO key buffers with g_malloc so a failed allocation cannot reach the following memcpy (REQ-MEM-04); remove the variable-length arrays and enforce -Werror=vla. Two of them were sized from remote input: the disco#info feature count and a chat message word length. The flag also caught a one-past-the-end write and a leak in the plugin autocompleter bindings (REQ-MEM-09)
This commit is contained in:
@@ -238,6 +238,8 @@ main(int argc, char* argv[])
|
||||
PROF_FUNC_TEST(presence_keeps_status),
|
||||
PROF_FUNC_TEST(presence_received),
|
||||
PROF_FUNC_TEST(presence_missing_resource_defaults),
|
||||
PROF_FUNC_TEST(presence_error_invalid_from_no_crash),
|
||||
PROF_FUNC_TEST(presence_subscription_invalid_from_no_crash),
|
||||
|
||||
/* Disconnect - clean session termination */
|
||||
PROF_FUNC_TEST(disconnect_ends_session),
|
||||
@@ -261,6 +263,7 @@ main(int argc, char* argv[])
|
||||
PROF_FUNC_TEST(message_send),
|
||||
PROF_FUNC_TEST(message_receive_console),
|
||||
PROF_FUNC_TEST(message_receive_chatwin),
|
||||
PROF_FUNC_TEST(message_invalid_from_silence_no_crash),
|
||||
|
||||
/* XEP-0359 disco gate for stanza-id trust */
|
||||
PROF_FUNC_TEST(stanza_id_dedup_fires_when_server_announces_sid0),
|
||||
|
||||
@@ -123,3 +123,29 @@ stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state)
|
||||
assert_false(prof_output_exact("Got a message with duplicate (server-generated) stanza-id"));
|
||||
prof_timeout_reset();
|
||||
}
|
||||
|
||||
/* Regression test for issue #148 (REQ-INP-01): with silence.non-roster
|
||||
* enabled the incoming-message filter dereferenced jid_create() without a
|
||||
* NULL check, so a message with an invalid 'from' crashed the client. */
|
||||
void
|
||||
message_invalid_from_silence_no_crash(void **state)
|
||||
{
|
||||
prof_connect();
|
||||
|
||||
prof_input("/silence on");
|
||||
assert_true(prof_output_exact("Block all messages from JIDs that are not in the roster enabled."));
|
||||
|
||||
stbbr_send(
|
||||
"<message to='stabber@localhost' from='bad@@jid' type='chat'>"
|
||||
"<body>should be dropped, not crash</body>"
|
||||
"</message>"
|
||||
);
|
||||
|
||||
/* client is still alive: a roster contact's message comes through */
|
||||
stbbr_send(
|
||||
"<message to='stabber@localhost' from='buddy1@localhost/mobile' type='chat'>"
|
||||
"<body>still alive</body>"
|
||||
"</message>"
|
||||
);
|
||||
assert_true(prof_output_exact("<< chat message: Buddy1/mobile (win 2)"));
|
||||
}
|
||||
|
||||
@@ -3,3 +3,4 @@ void message_receive_console(void **state);
|
||||
void message_receive_chatwin(void **state);
|
||||
void stanza_id_dedup_fires_when_server_announces_sid0(void **state);
|
||||
void stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state);
|
||||
void message_invalid_from_silence_no_crash(void **state);
|
||||
|
||||
@@ -289,3 +289,53 @@ presence_missing_resource_defaults(void **state)
|
||||
|
||||
assert_true(prof_output_exact("__prof_default (15), online"));
|
||||
}
|
||||
|
||||
/* Regression tests for issue #148 (REQ-INP-01): presence handlers used to
|
||||
* dereference jid_create() results without a NULL check, so a stanza with a
|
||||
* missing or invalid 'from' crashed the client. */
|
||||
void
|
||||
presence_error_invalid_from_no_crash(void **state)
|
||||
{
|
||||
prof_connect();
|
||||
|
||||
/* MUC join error with an invalid 'from' */
|
||||
stbbr_send(
|
||||
"<presence to='stabber@localhost' from='bad@@room' type='error'>"
|
||||
"<x xmlns='http://jabber.org/protocol/muc'/>"
|
||||
"<error type='cancel'><not-allowed xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>"
|
||||
"</presence>"
|
||||
);
|
||||
|
||||
/* same, with no 'from' at all */
|
||||
stbbr_send(
|
||||
"<presence to='stabber@localhost' type='error'>"
|
||||
"<x xmlns='http://jabber.org/protocol/muc'/>"
|
||||
"<error type='cancel'><not-allowed xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>"
|
||||
"</presence>"
|
||||
);
|
||||
|
||||
/* client is still alive and processing stanzas */
|
||||
stbbr_send(
|
||||
"<presence to='stabber@localhost' from='buddy1@localhost/mobile'>"
|
||||
"<status>still alive</status>"
|
||||
"</presence>"
|
||||
);
|
||||
assert_true(prof_output_exact("Buddy1 (mobile) is online, \"still alive\""));
|
||||
}
|
||||
|
||||
void
|
||||
presence_subscription_invalid_from_no_crash(void **state)
|
||||
{
|
||||
prof_connect();
|
||||
|
||||
stbbr_send("<presence to='stabber@localhost' from='bad@@jid' type='subscribed'/>");
|
||||
stbbr_send("<presence to='stabber@localhost' from='bad@@jid' type='unsubscribed'/>");
|
||||
stbbr_send("<presence to='stabber@localhost' from='bad@@jid' type='subscribe'/>");
|
||||
|
||||
stbbr_send(
|
||||
"<presence to='stabber@localhost' from='buddy1@localhost/mobile'>"
|
||||
"<status>still alive</status>"
|
||||
"</presence>"
|
||||
);
|
||||
assert_true(prof_output_exact("Buddy1 (mobile) is online, \"still alive\""));
|
||||
}
|
||||
|
||||
@@ -13,3 +13,5 @@ void presence_includes_priority(void **state);
|
||||
void presence_keeps_status(void **state);
|
||||
void presence_received(void **state);
|
||||
void presence_missing_resource_defaults(void **state);
|
||||
void presence_error_invalid_from_no_crash(void **state);
|
||||
void presence_subscription_invalid_from_no_crash(void **state);
|
||||
|
||||
Reference in New Issue
Block a user