fix: address issue #112 follow-ups (OTR strip, presence UAF, OMEMO load)
All checks were successful
CI Code / Check coding style (pull_request) Successful in 33s
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m46s
CI Code / Linux (arch) (pull_request) Successful in 5m36s
CI Code / Linux (debian) (pull_request) Successful in 6m49s
CI Code / Code Coverage (pull_request) Successful in 6m57s
All checks were successful
CI Code / Check coding style (pull_request) Successful in 33s
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m46s
CI Code / Linux (arch) (pull_request) Successful in 5m36s
CI Code / Linux (debian) (pull_request) Successful in 6m49s
CI Code / Code Coverage (pull_request) Successful in 6m57s
- otr: strip the OTR whitespace tag by shifting the full remaining tail (including the trailing NUL) instead of tag_length bytes, which duplicated the body for messages longer than the tag (#8). - xmpp: snapshot resource name/status/priority/presence before connection_add_available_resource() transfers ownership of the Resource, so the later reads can no longer become a use-after-free (#9). - omemo: propagate _omemo_finalize_identity_load failure on connect — mark the context unloaded, log it, surface a cons_show_error, and stop instead of silently leaving OMEMO unavailable (#10). Refs #112
This commit is contained in:
@@ -260,7 +260,12 @@ omemo_on_connect(ProfAccount* account)
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
_omemo_finalize_identity_load(account);
|
if (!_omemo_finalize_identity_load(account)) {
|
||||||
|
omemo_ctx.loaded = FALSE;
|
||||||
|
log_error("[OMEMO] failed to load OMEMO state from disk for %s", account->jid);
|
||||||
|
cons_show_error("OMEMO: could not load encryption state from disk; OMEMO will be unavailable this session.");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
wins_omemo_trust_changed(NULL);
|
wins_omemo_trust_changed(NULL);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -278,7 +278,10 @@ otr_on_message_recv(const char* const barejid, const char* const resource, const
|
|||||||
if (strstr(message, OTRL_MESSAGE_TAG_V2) && strstr(message, OTRL_MESSAGE_TAG_V1)) {
|
if (strstr(message, OTRL_MESSAGE_TAG_V2) && strstr(message, OTRL_MESSAGE_TAG_V1)) {
|
||||||
tag_length = 32;
|
tag_length = 32;
|
||||||
}
|
}
|
||||||
memmove(whitespace_base, whitespace_base + tag_length, tag_length);
|
// Shift the remainder of the message (including the trailing NUL)
|
||||||
|
// left over the tag, not just the next tag_length bytes — otherwise
|
||||||
|
// the tail is left in place and the body is duplicated/truncated.
|
||||||
|
memmove(whitespace_base, whitespace_base + tag_length, strlen(whitespace_base + tag_length) + 1);
|
||||||
char* otr_query_message = otr_start_query();
|
char* otr_query_message = otr_start_query();
|
||||||
cons_show("OTR Whitespace pattern detected. Attempting to start OTR session…");
|
cons_show("OTR Whitespace pattern detected. Attempting to start OTR session…");
|
||||||
free(message_send_chat_otr(barejid, otr_query_message, FALSE, NULL));
|
free(message_send_chat_otr(barejid, otr_query_message, FALSE, NULL));
|
||||||
|
|||||||
@@ -621,15 +621,23 @@ _available_handler(xmpp_stanza_t* const stanza)
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (g_strcmp0(xmpp_presence->jid->barejid, my_jid->barejid) == 0) {
|
if (g_strcmp0(xmpp_presence->jid->barejid, my_jid->barejid) == 0) {
|
||||||
connection_add_available_resource(resource);
|
// Snapshot everything we read from `resource` before ownership is
|
||||||
|
// transferred to the available-resources table; dereferencing it
|
||||||
|
// afterwards would be a use-after-free should the same resource name
|
||||||
|
// ever be re-added between the transfer and the reads below.
|
||||||
Resource* resource_for_roster = resource_copy(resource);
|
Resource* resource_for_roster = resource_copy(resource);
|
||||||
|
auto_gchar gchar* resource_name = g_strdup(resource->name);
|
||||||
|
auto_gchar gchar* resource_status = resource->status ? g_strdup(resource->status) : NULL;
|
||||||
|
int resource_priority = resource->priority;
|
||||||
|
resource_presence_t resource_presence = resource->presence;
|
||||||
|
connection_add_available_resource(resource);
|
||||||
sv_ev_contact_online(xmpp_presence->jid->barejid, resource_for_roster, xmpp_presence->last_activity, pgpsig);
|
sv_ev_contact_online(xmpp_presence->jid->barejid, resource_for_roster, xmpp_presence->last_activity, pgpsig);
|
||||||
const char* account_name = session_get_account_name();
|
const char* account_name = session_get_account_name();
|
||||||
int max_sessions = accounts_get_max_sessions(account_name);
|
int max_sessions = accounts_get_max_sessions(account_name);
|
||||||
if (max_sessions > 0) {
|
if (max_sessions > 0) {
|
||||||
auto_gchar gchar* cur_resource = accounts_get_resource(account_name);
|
auto_gchar gchar* cur_resource = accounts_get_resource(account_name);
|
||||||
int res_count = connection_count_available_resources();
|
int res_count = connection_count_available_resources();
|
||||||
if (res_count > max_sessions && g_strcmp0(cur_resource, resource->name)) {
|
if (res_count > max_sessions && g_strcmp0(cur_resource, resource_name)) {
|
||||||
ProfWin* console = wins_get_console();
|
ProfWin* console = wins_get_console();
|
||||||
ProfWin* current_window = wins_get_current();
|
ProfWin* current_window = wins_get_current();
|
||||||
auto_gchar gchar* message = g_strdup_printf("Max sessions alarm! (%d/%d devices in use)", res_count, max_sessions);
|
auto_gchar gchar* message = g_strdup_printf("Max sessions alarm! (%d/%d devices in use)", res_count, max_sessions);
|
||||||
@@ -639,14 +647,14 @@ _available_handler(xmpp_stanza_t* const stanza)
|
|||||||
}
|
}
|
||||||
notify(message, 10000, "Security alert");
|
notify(message, 10000, "Security alert");
|
||||||
|
|
||||||
const char* resource_presence = string_from_resource_presence(resource->presence);
|
const char* resource_presence_str = string_from_resource_presence(resource_presence);
|
||||||
win_print(console, THEME_DEFAULT, "|", "New device info: \n %s (%d), %s", resource->name, resource->priority, resource_presence);
|
win_print(console, THEME_DEFAULT, "|", "New device info: \n %s (%d), %s", resource_name, resource_priority, resource_presence_str);
|
||||||
|
|
||||||
if (resource->status) {
|
if (resource_status) {
|
||||||
win_append(console, THEME_DEFAULT, ", \"%s\"", resource->status);
|
win_append(console, THEME_DEFAULT, ", \"%s\"", resource_status);
|
||||||
}
|
}
|
||||||
win_appendln(console, THEME_DEFAULT, "");
|
win_appendln(console, THEME_DEFAULT, "");
|
||||||
auto_jid Jid* jidp = jid_create_from_bare_and_resource(my_jid->barejid, resource->name);
|
auto_jid Jid* jidp = jid_create_from_bare_and_resource(my_jid->barejid, resource_name);
|
||||||
EntityCapabilities* caps = caps_lookup(jidp->fulljid);
|
EntityCapabilities* caps = caps_lookup(jidp->fulljid);
|
||||||
|
|
||||||
if (caps) {
|
if (caps) {
|
||||||
|
|||||||
Reference in New Issue
Block a user