Fix: issue #112 follow-ups — OTR strip, presence UAF, OMEMO load #130

Manually merged
jabber.developer merged 6 commits from fix/issue-112-followups into master 2026-06-20 10:30:41 +00:00
19 changed files with 208 additions and 48 deletions
Showing only changes of commit f16b26bd6d - Show all commits

View File

@@ -10066,7 +10066,7 @@ cmd_vcard_remove(ProfWin* window, const char* const command, gchar** args)
int index;
auto_gchar gchar* err_msg = NULL;
if (!strtoi_range(args[1], &index, 0, INT_MAX, &err_msg)) {
cons_show("%s", err_msg);
cons_show_error("%s", err_msg);
jabber.developer marked this conversation as resolved Outdated

I think that cons_show_error would be more appropriate.

I think that `cons_show_error` would be more appropriate.
return TRUE;
}
vcard_user_remove_element((unsigned int)index);

View File

@@ -309,6 +309,16 @@ str_replace(const char* string, const char* substr,
return newstr;
}
gsize
g_diff_to_gsize(const void* end, const void* start)
{
if (end < start) {
log_error("g_diff_to_gsize: end < start (%p < %p)", end, start);
return 0;
}
return (gsize)((const char*)end - (const char*)start);
}
gboolean
strtoi_range(const char* str, int* saveptr, int min, int max, gchar** err_msg)
{
@@ -690,19 +700,18 @@ get_file_paths_recursive(const char* path, GSList** contents)
}
gchar*
get_random_string(int length)
get_random_string(size_t length)
{
g_assert(length >= 0);
GRand* prng;
gchar* rand;
gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
int endrange = sizeof(alphabet) - 1;
rand = g_malloc0((gsize)length + 1);
rand = g_malloc0(length + 1);
prng = g_rand_new();
for (int i = 0; i < length; i++) {
for (size_t i = 0; i < length; i++) {
rand[i] = alphabet[g_rand_int_range(prng, 0, endrange)];
}
g_rand_free(prng);

View File

@@ -158,14 +158,7 @@ gboolean create_dir(const char* name);
gboolean copy_file(const char* const src, const char* const target, const gboolean overwrite_existing);
char* str_replace(const char* string, const char* substr, const char* replacement);
gboolean strtoi_range(const char* str, int* saveptr, int min, int max, char** err_msg);
// Pointer-diff to gsize for callers that know end >= start by construction.
static inline gsize
g_diff_to_gsize(const void* end, const void* start)
{
g_assert(end >= start);
return (gsize)((const char*)end - (const char*)start);
}
gsize g_diff_to_gsize(const void* end, const void* start);
int utf8_display_len(const char* const str);
gchar* str_xml_sanitize(const char* const str);
@@ -186,7 +179,7 @@ int is_regular_file(const char* path);
int is_dir(const char* path);
void get_file_paths_recursive(const char* directory, GSList** contents);
gchar* get_random_string(int length);
gchar* get_random_string(size_t length);
gboolean call_external(gchar** argv);
gchar** format_call_external_argv(const char* template_fmt, const char* url, const char* filename);

View File

@@ -278,9 +278,7 @@ 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)) {
tag_length = 32;
}
// 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.
// Move the rest of the message (with NUL) over the tag.
jabber.developer marked this conversation as resolved Outdated

The comment explains the nature of change, which is a known antipattern.

The comment explains the nature of change, which is a known antipattern.
memmove(whitespace_base, whitespace_base + tag_length, strlen(whitespace_base + tag_length) + 1);
char* otr_query_message = otr_start_query();
cons_show("OTR Whitespace pattern detected. Attempting to start OTR session…");

View File

@@ -621,10 +621,7 @@ _available_handler(xmpp_stanza_t* const stanza)
}
if (g_strcmp0(xmpp_presence->jid->barejid, my_jid->barejid) == 0) {
// 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.
// Copy what we read before connection_add_available_resource() takes ownership.
jabber.developer marked this conversation as resolved Outdated

nit: Ideally we want more concise 1-2 line comments instead. Further discussion is other required.

nit: Ideally we want more concise 1-2 line comments instead. Further discussion is other required.
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;

View File

@@ -119,10 +119,9 @@ _create_dir(const char* name)
gboolean
_mkdir_recursive(const char* dir)
{
size_t i;
gboolean result = TRUE;
jabber.developer marked this conversation as resolved Outdated

Why is it even defined out of loop scope? We can move it inside of the for body for readability.

Why is it even defined out of loop scope? We can move it inside of the `for` body for readability.
for (i = 1; i <= strlen(dir); i++) {
for (size_t i = 1; i <= strlen(dir); i++) {
if (dir[i] == '/' || dir[i] == '\0') {
gchar* next_dir = g_strndup(dir, i);
result = _create_dir(next_dir);