Merge pull request #2102 from profanity-im/msgjid

Improve JID validation and fix a bug when loading database messages

src/command/cmd_funcs.c

@@ -2155,7 +2155,11 @@ cmd_msg(ProfWin* window, const char* const command, gchar** args)
             barejid = usr;
         }
 
-        _cmd_msg_chatwin(barejid, msg);
+        if (jid_is_valid_user_jid(barejid)) {
+            _cmd_msg_chatwin(barejid, msg);
+        } else {
+            cons_show_error("Not a contact or valid JID: %s", barejid);
+        }
 
         return TRUE;
     }

src/database.c

@@ -309,6 +309,9 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las
 
         msg->stanzaid = _db_strdup(archive_id);
         msg->timestamp = g_date_time_new_from_iso8601(date, NULL);
+    } else {
+        message_free(msg);
+        msg = NULL;
     }
     sqlite3_finalize(stmt);
 

src/xmpp/jid.c

@@ -43,65 +43,157 @@
 #include "common.h"
 #include "xmpp/jid.h"
 
+#define JID_MAX_PART_LEN  1023
+#define JID_MAX_TOTAL_LEN 3071
+
+static gboolean
+_is_invalid_local_char(gunichar c)
+{
+    // RFC 6122: " & ' / : < > @
+    // Also space is forbidden.
+    return (c == ' ' || c == '"' || c == '&' || c == '\'' || c == '/' || c == ':' || c == '<' || c == '>' || c == '@');
+}
+
 Jid*
 jid_create(const gchar* const str)
 {
-    Jid* result = NULL;
+    if (!jid_is_valid(str)) {
+        return NULL;
+    }
 
-    /* if str is NULL g_strdup returns NULL */
     gchar* trimmed = g_strdup(str);
     if (trimmed == NULL) {
         return NULL;
     }
 
-    if (strlen(trimmed) == 0) {
-        g_free(trimmed);
-        return NULL;
-    }
-
-    if (g_str_has_prefix(trimmed, "/") || g_str_has_prefix(trimmed, "@")) {
-        g_free(trimmed);
-        return NULL;
-    }
-
-    if (!g_utf8_validate(trimmed, -1, NULL)) {
-        g_free(trimmed);
-        return NULL;
-    }
-
-    result = g_new0(Jid, 1);
+    Jid* result = g_new0(Jid, 1);
     result->refcnt = 1;
+    result->str = trimmed;
 
-    gchar* atp = g_utf8_strchr(trimmed, -1, '@');
     gchar* slashp = g_utf8_strchr(trimmed, -1, '/');
-    gchar* domain_start = trimmed;
+    auto_gchar gchar* bare = NULL;
+
+    if (slashp) {
+        if (strlen(slashp + 1) > 0) {
+            result->resourcepart = g_strdup(slashp + 1);
+            result->fulljid = g_strdup(trimmed);
+        }
+        bare = g_utf8_substring(trimmed, 0, g_utf8_pointer_to_offset(trimmed, slashp));
+    } else {
+        bare = g_strdup(trimmed);
+    }
+
+    gchar* atp = g_utf8_strchr(bare, -1, '@');
+    gchar* domain_start = bare;
 
     if (atp) {
-        result->localpart = g_utf8_substring(trimmed, 0, g_utf8_pointer_to_offset(trimmed, atp));
+        if (g_utf8_pointer_to_offset(bare, atp) > 0) {
+            result->localpart = g_utf8_substring(bare, 0, g_utf8_pointer_to_offset(bare, atp));
+        }
         domain_start = atp + 1;
     }
 
-    if (slashp) {
-        result->resourcepart = g_strdup(slashp + 1);
-        result->domainpart = g_utf8_substring(domain_start, 0, g_utf8_pointer_to_offset(domain_start, slashp));
-        auto_gchar gchar* barejidraw = g_utf8_substring(trimmed, 0, g_utf8_pointer_to_offset(trimmed, slashp));
-        result->barejid = g_utf8_strdown(barejidraw, -1);
-        result->fulljid = g_strdup(trimmed);
-    } else {
+    if (strlen(domain_start) > 0) {
         result->domainpart = g_strdup(domain_start);
-        result->barejid = g_utf8_strdown(trimmed, -1);
     }
 
-    if (result->domainpart == NULL) {
-        jid_destroy(result);
-        return NULL;
-    }
-
-    result->str = trimmed;
+    result->barejid = g_utf8_strdown(bare, -1);
 
     return result;
 }
 
+gboolean
+jid_is_valid(const gchar* const str)
+{
+    if (str == NULL) {
+        return FALSE;
+    }
+
+    size_t total_len = strlen(str);
+    if (total_len == 0 || total_len > JID_MAX_TOTAL_LEN) {
+        return FALSE;
+    }
+
+    if (!g_utf8_validate(str, -1, NULL)) {
+        return FALSE;
+    }
+
+    const gchar* slash = g_utf8_strchr(str, -1, '/');
+    const gchar* at = NULL;
+
+    // Find the first '@' that is NOT in the resourcepart
+    // and ensure there is at most one '@' in the bare JID
+    const gchar* p = str;
+    while (*p && (!slash || p < slash)) {
+        if (g_utf8_get_char(p) == '@') {
+            if (at == NULL) {
+                at = p;
+            } else {
+                // More than one '@' in bare JID
+                return FALSE;
+            }
+        }
+        p = g_utf8_next_char(p);
+    }
+
+    const gchar* domain_start = str;
+    size_t domain_len = 0;
+
+    // Localpart validation
+    if (at) {
+        size_t local_len = at - str;
+        if (local_len == 0 || local_len > JID_MAX_PART_LEN) {
+            return FALSE;
+        }
+
+        const gchar* lp = str;
+        while (lp < at) {
+            gunichar c = g_utf8_get_char(lp);
+            if (_is_invalid_local_char(c)) {
+                return FALSE;
+            }
+            lp = g_utf8_next_char(lp);
+        }
+        domain_start = at + 1;
+    }
+
+    // Resourcepart validation if present
+    if (slash) {
+        domain_len = slash - domain_start;
+        size_t resource_len = strlen(slash + 1);
+        if (resource_len > JID_MAX_PART_LEN) {
+            return FALSE;
+        }
+    } else {
+        domain_len = strlen(domain_start);
+    }
+
+    // Domainpart validation
+    if (domain_len == 0 || domain_len > JID_MAX_PART_LEN) {
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+gboolean
+jid_is_valid_user_jid(const gchar* const str)
+{
+    if (!jid_is_valid(str)) {
+        return FALSE;
+    }
+
+    const gchar* slash = g_utf8_strchr(str, -1, '/');
+    const gchar* at = g_utf8_strchr(str, -1, '@');
+
+    // A user JID must have an '@' before any '/'
+    if (at && (!slash || at < slash)) {
+        return TRUE;
+    }
+
+    return FALSE;
+}
+
 Jid*
 jid_create_from_bare_and_resource(const gchar* const barejid, const gchar* const resource)
 {
@@ -158,7 +250,11 @@ gchar*
 create_fulljid(const gchar* const barejid, const gchar* const resource)
 {
     auto_gchar gchar* barejidlower = g_utf8_strdown(barejid, -1);
-    return g_strdup_printf("%s/%s", barejidlower, resource);
+    if (resource && strlen(resource) > 0) {
+        return g_strdup_printf("%s/%s", barejidlower, resource);
+    } else {
+        return g_strdup(barejidlower);
+    }
 }
 
 /*

src/xmpp/jid.h

@@ -52,6 +52,8 @@ struct jid_t
 typedef struct jid_t Jid;
 
 Jid* jid_create(const gchar* const str);
+gboolean jid_is_valid(const gchar* const str);
+gboolean jid_is_valid_user_jid(const gchar* const str);
 Jid* jid_create_from_bare_and_resource(const gchar* const barejid, const gchar* const resource);
 void jid_destroy(Jid* jid);
 void jid_ref(Jid* jid);

tests/unittests/unittests.c

@@ -150,6 +150,18 @@ main(int argc, char* argv[])
         cmocka_unit_test(jid_create__returns__correct_parts_with_trailing_slash),
         cmocka_unit_test(jid_fulljid_or_barejid__returns__fulljid_when_exists),
         cmocka_unit_test(jid_fulljid_or_barejid__returns__barejid_when_fulljid_not_exists),
+        cmocka_unit_test(jid_create__returns__bare_from_trailing_slash),
+        cmocka_unit_test(jid_create__returns__null_from_invalid_localpart_chars),
+        cmocka_unit_test(jid_create__returns__null_from_empty_parts),
+        cmocka_unit_test(jid_create__returns__null_from_multiple_at_in_bare),
+        cmocka_unit_test(jid_create__returns__correct_parts_with_at_in_resource_only),
+        cmocka_unit_test(jid_is_valid_user_jid__is__true_for_valid_user_jid),
+        cmocka_unit_test(jid_is_valid_user_jid__is__false_for_domain_jid),
+        cmocka_unit_test(jid_is_valid_user_jid__is__false_for_invalid_jid),
+        cmocka_unit_test(jid_is_valid__is__true_for_valid_jid),
+        cmocka_unit_test(jid_is_valid__is__false_for_invalid_jid),
+        cmocka_unit_test(jid_is_valid__is__false_for_null),
+        cmocka_unit_test(jid_is_valid__is__false_for_empty_string),
 
         cmocka_unit_test(parse_args__returns__null_from_null),
         cmocka_unit_test(parse_args__returns__null_from_empty),

tests/unittests/xmpp/test_jid.c

@@ -234,3 +234,131 @@ jid_fulljid_or_barejid__returns__barejid_when_fulljid_not_exists(void** state)
 
     jid_destroy(jid);
 }
+
+void
+jid_create__returns__bare_from_trailing_slash(void** state)
+{
+    Jid* result = jid_create("myuser@mydomain/");
+    assert_non_null(result);
+    assert_string_equal("myuser@mydomain", result->barejid);
+    assert_null(result->resourcepart);
+    jid_destroy(result);
+}
+
+void
+jid_create__returns__null_from_invalid_localpart_chars(void** state)
+{
+    // Space
+    Jid* result = jid_create("user name@domain.com");
+    assert_null(result);
+
+    // Double quote
+    result = jid_create("user\"name@domain.com");
+    assert_null(result);
+
+    // Ampersand
+    result = jid_create("user&name@domain.com");
+    assert_null(result);
+
+    // Single quote
+    result = jid_create("user'name@domain.com");
+    assert_null(result);
+
+    // Colon
+    result = jid_create("user:name@domain.com");
+    assert_null(result);
+
+    // Less than
+    result = jid_create("user<name@domain.com");
+    assert_null(result);
+
+    // Greater than
+    result = jid_create("user>name@domain.com");
+    assert_null(result);
+}
+
+void
+jid_create__returns__null_from_empty_parts(void** state)
+{
+    // Empty local
+    Jid* result = jid_create("@domain.com");
+    assert_null(result);
+
+    // Empty domain (trailing @)
+    result = jid_create("user@");
+    assert_null(result);
+
+    // Empty domain in full jid
+    result = jid_create("user@/resource");
+    assert_null(result);
+}
+
+void
+jid_create__returns__null_from_multiple_at_in_bare(void** state)
+{
+    Jid* result = jid_create("user@domain@other.com");
+    assert_null(result);
+}
+
+void
+jid_create__returns__correct_parts_with_at_in_resource_only(void** state)
+{
+    // domain.com/user@host (no localpart)
+    Jid* result = jid_create("domain.com/user@host");
+    assert_non_null(result);
+    assert_null(result->localpart);
+    assert_string_equal("domain.com", result->domainpart);
+    assert_string_equal("user@host", result->resourcepart);
+    jid_destroy(result);
+}
+
+void
+jid_is_valid_user_jid__is__true_for_valid_user_jid(void** state)
+{
+    assert_true(jid_is_valid_user_jid("myuser@mydomain/laptop"));
+    assert_true(jid_is_valid_user_jid("myuser@mydomain"));
+}
+
+void
+jid_is_valid_user_jid__is__false_for_domain_jid(void** state)
+{
+    assert_false(jid_is_valid_user_jid("mydomain/laptop"));
+    assert_false(jid_is_valid_user_jid("mydomain"));
+}
+
+void
+jid_is_valid_user_jid__is__false_for_invalid_jid(void** state)
+{
+    assert_false(jid_is_valid_user_jid("@mydomain"));
+    assert_false(jid_is_valid_user_jid("/laptop"));
+    assert_false(jid_is_valid_user_jid(NULL));
+    assert_false(jid_is_valid_user_jid(""));
+}
+
+void
+jid_is_valid__is__true_for_valid_jid(void** state)
+{
+    assert_true(jid_is_valid("myuser@mydomain/laptop"));
+    assert_true(jid_is_valid("myuser@mydomain"));
+    assert_true(jid_is_valid("mydomain/laptop"));
+    assert_true(jid_is_valid("mydomain"));
+}
+
+void
+jid_is_valid__is__false_for_invalid_jid(void** state)
+{
+    assert_false(jid_is_valid("@mydomain"));
+    assert_false(jid_is_valid("/laptop"));
+}
+
+void
+jid_is_valid__is__false_for_null(void** state)
+{
+    assert_false(jid_is_valid(NULL));
+}
+
+void
+jid_is_valid__is__false_for_empty_string(void** state)
+{
+    assert_false(jid_is_valid(""));
+}

tests/unittests/xmpp/test_jid.h

@@ -26,5 +26,17 @@ void jid_create__returns__correct_parts_with_at_and_slash_in_resource(void** sta
 void jid_create__returns__correct_parts_with_trailing_slash(void** state);
 void jid_fulljid_or_barejid__returns__fulljid_when_exists(void** state);
 void jid_fulljid_or_barejid__returns__barejid_when_fulljid_not_exists(void** state);
+void jid_create__returns__bare_from_trailing_slash(void** state);
+void jid_create__returns__null_from_invalid_localpart_chars(void** state);
+void jid_create__returns__null_from_empty_parts(void** state);
+void jid_create__returns__null_from_multiple_at_in_bare(void** state);
+void jid_create__returns__correct_parts_with_at_in_resource_only(void** state);
+void jid_is_valid_user_jid__is__true_for_valid_user_jid(void** state);
+void jid_is_valid_user_jid__is__false_for_domain_jid(void** state);
+void jid_is_valid_user_jid__is__false_for_invalid_jid(void** state);
+void jid_is_valid__is__true_for_valid_jid(void** state);
+void jid_is_valid__is__false_for_invalid_jid(void** state);
+void jid_is_valid__is__false_for_null(void** state);
+void jid_is_valid__is__false_for_empty_string(void** state);
 
 #endif