no-DB mode implementation #94

Manually merged
jabber.developer merged 34 commits from feat/no-db-mode into master 2026-05-05 19:32:42 +00:00
14 changed files with 2767 additions and 645 deletions
Showing only changes of commit 89b90e5cf2 - Show all commits

View File

@@ -56,7 +56,8 @@ typedef enum {
} integrity_level_t; } integrity_level_t;
// A single integrity issue found during verification // A single integrity issue found during verification
typedef struct { typedef struct
{
integrity_level_t level; integrity_level_t level;
char* file; char* file;
int line; int line;
@@ -66,7 +67,8 @@ typedef struct {
void integrity_issue_free(integrity_issue_t* issue); void integrity_issue_free(integrity_issue_t* issue);
// Backend vtable: pluggable storage backends implement this interface // Backend vtable: pluggable storage backends implement this interface
typedef struct db_backend_t { typedef struct db_backend_t
{
const char* name; const char* name;
gboolean (*init)(ProfAccount* account); gboolean (*init)(ProfAccount* account);
void (*close)(void); void (*close)(void);

View File

@@ -24,6 +24,8 @@
#include "config.h" #include "config.h"
#include <sys/stat.h> #include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <glib.h> #include <glib.h>
#include <glib/gstdio.h> #include <glib/gstdio.h>
#include <stdio.h> #include <stdio.h>
@@ -42,6 +44,8 @@
#define DIR_FLATLOG "flatlog" #define DIR_FLATLOG "flatlog"
#define FLATFILE_HEADER "# profanity chat log — UTF-8, LF line endings\n# vim: set fileencoding=utf-8 fileformat=unix :\n" #define FLATFILE_HEADER "# profanity chat log — UTF-8, LF line endings\n# vim: set fileencoding=utf-8 fileformat=unix :\n"
#define FF_MAX_LINE_LEN (10 * 1024 * 1024) /* 10 MB — reject lines longer than this */
#define FF_MAX_LMC_DEPTH 100 /* max correction chain depth */
// Account JID stored during init for path construction // Account JID stored during init for path construction
static char* g_flatfile_account_jid = NULL; static char* g_flatfile_account_jid = NULL;
@@ -112,12 +116,48 @@ _ff_get_message_enc_type(const char* const encstr)
// --- Path helpers --- // --- Path helpers ---
jabber.developer marked this conversation as resolved
Review

nit: I suggest this version for readability (avoid magic number of 3 and 4).

for (int i = 0; parts[i]; i++) {
    if (g_str_has_prefix(parts[i], "id:") && !stanza_id) {
        stanza_id = ff_unescape_meta_value(parts[i] + strlen("id:"));
    } else if (g_str_has_prefix(parts[i], "aid:") && !archive_id) {
        archive_id = ff_unescape_meta_value(parts[i] + strlen("aid:"));
    }
}

Ideally, we can also move "id:" and "aid:" into separate constants. It would also increase robustness in case of prefix changes.

nit: I suggest this version for readability (avoid magic number of 3 and 4). ```c for (int i = 0; parts[i]; i++) { if (g_str_has_prefix(parts[i], "id:") && !stanza_id) { stanza_id = ff_unescape_meta_value(parts[i] + strlen("id:")); } else if (g_str_has_prefix(parts[i], "aid:") && !archive_id) { archive_id = ff_unescape_meta_value(parts[i] + strlen("aid:")); } } ``` Ideally, we can also move "id:" and "aid:" into separate constants. It would also increase robustness in case of prefix changes.

Corrected

Corrected
// Replace '@' with '_at_' in JID for filesystem-safe directory names // Sanitise a JID for use as a directory name.
// 1. Replace '@' with '_at_'
// 2. Reject / strip path-separator and traversal characters: '/', '\0', '..'
// This prevents a malicious federated JID like "../../../tmp/pwned" from
// escaping the log directory tree.
static char* static char*
jabber.developer marked this conversation as resolved
Review

what if *(close + 1) overflows?

what if `*(close + 1)` overflows?

Corrected

Corrected
_ff_jid_to_dir(const char* jid) _ff_jid_to_dir(const char* jid)
jabber.developer marked this conversation as resolved
Review

again no overflow check

again no overflow check

Corrected

Corrected
{ {
if (!jid) return NULL; if (!jid || jid[0] == '\0')
return str_replace(jid, "@", "_at_"); return NULL;
jabber.developer marked this conversation as resolved
Review

Instead of loop, we can do something like that:
g_utf8_strchr(sender_start, colonspace, '/')

Instead of loop, we can do something like that: `g_utf8_strchr(sender_start, colonspace, '/')`
// Reject embedded null bytes (strlen already stops, but be explicit)
// Replace '@' first
char* step1 = str_replace(jid, "@", "_at_");
if (!step1)
return NULL;
// Replace '/' and '\\' with '_' to prevent path traversal
GString* out = g_string_sized_new(strlen(step1));
for (const char* p = step1; *p; p++) {
if (*p == '/' || *p == '\\') {
g_string_append_c(out, '_');
} else {
g_string_append_c(out, *p);
}
}
free(step1);
// Collapse any remaining ".." sequences to "__" (belt-and-suspenders)
char* result = g_string_free(out, FALSE);
char* dotdot;
while ((dotdot = strstr(result, "..")) != NULL) {
dotdot[0] = '_';
dotdot[1] = '_';
}
// Reject empty result
if (result[0] == '\0') {
g_free(result);
return NULL;
}
return result;
} }
// Get the base directory for a contact's logs: // Get the base directory for a contact's logs:
jabber.developer marked this conversation as resolved Outdated

Please, extract bom-check to a different method. it is violates DRY and SRP. database_flatfile_validate.c:123

Please, extract bom-check to a different method. it is violates DRY and SRP. `database_flatfile_validate.c:123`

Corrected

Corrected
@@ -125,7 +165,8 @@ _ff_jid_to_dir(const char* jid)
static char* static char*
_ff_get_contact_dir(const char* contact_barejid) _ff_get_contact_dir(const char* contact_barejid)
{ {
if (!g_flatfile_account_jid || !contact_barejid) return NULL; if (!g_flatfile_account_jid || !contact_barejid)
return NULL;
auto_gchar gchar* data_path = files_get_data_path(DIR_FLATLOG); auto_gchar gchar* data_path = files_get_data_path(DIR_FLATLOG);
auto_gchar gchar* my_dir = _ff_jid_to_dir(g_flatfile_account_jid); auto_gchar gchar* my_dir = _ff_jid_to_dir(g_flatfile_account_jid);
@@ -141,18 +182,26 @@ static char*
_ff_get_log_path(const char* contact_barejid, GDateTime* dt) _ff_get_log_path(const char* contact_barejid, GDateTime* dt)
{ {
auto_gchar gchar* contact_dir = _ff_get_contact_dir(contact_barejid); auto_gchar gchar* contact_dir = _ff_get_contact_dir(contact_barejid);
if (!contact_dir) return NULL; if (!contact_dir)
return NULL;
auto_gchar gchar* date_str = g_date_time_format(dt, "%Y_%m_%d"); auto_gchar gchar* date_str = g_date_time_format(dt, "%Y_%m_%d");
char* result = g_strdup_printf("%s/%s.log", contact_dir, date_str); char* result = g_strdup_printf("%s/%s.log", contact_dir, date_str);
return result; return result;
} }
// Ensure the directory exists, create if needed // Ensure the directory exists, create if needed.
// Refuses to follow symlinks at the final component.
static gboolean static gboolean
_ff_ensure_dir(const char* path) _ff_ensure_dir(const char* path)
{ {
if (g_file_test(path, G_FILE_TEST_IS_DIR)) { if (g_file_test(path, G_FILE_TEST_IS_DIR)) {
// Verify it's not a symlink
jabber.developer marked this conversation as resolved Outdated

please add comment explaining purpose

please add comment explaining purpose

Corrected

Corrected
struct stat st;
if (g_lstat(path, &st) == 0 && S_ISLNK(st.st_mode)) {
log_error("flatfile: directory path is a symlink, refusing: %s", path);
return FALSE;
}
jabber.developer marked this conversation as resolved Outdated

Are !dt and !space cases ok to be ignored?

Are `!dt` and `!space` cases ok to be ignored?

Postponed — discussion. Current behaviour is "skip silently" (lines outside index step are not indexed); this is by design. No specific change requested.

Postponed — discussion. Current behaviour is "skip silently" (lines outside index step are not indexed); this is by design. No specific change requested.
return TRUE; return TRUE;
} }
jabber.developer marked this conversation as resolved Outdated

please, consider reduction of nesting

please, consider reduction of nesting

Corrected

Corrected
if (g_mkdir_with_parents(path, S_IRWXU) != 0) { if (g_mkdir_with_parents(path, S_IRWXU) != 0) {
@@ -162,11 +211,192 @@ _ff_ensure_dir(const char* path)
return TRUE; return TRUE;
} }
// --- Escape / unescape helpers ---
//
// Message text and metadata values from remote peers can contain arbitrary
// characters including newlines, pipes and brackets. Without escaping, a
// crafted message could inject fake log lines (log injection / format
// injection). We escape on write and unescape on read.
// Escape message body: \ -> \\, \n -> \n literal, \r -> \r literal
static char*
_ff_escape_message(const char* text)
{
if (!text)
return g_strdup("");
GString* out = g_string_sized_new(strlen(text));
for (const char* p = text; *p; p++) {
switch (*p) {
case '\\':
g_string_append(out, "\\\\");
break;
case '\n':
g_string_append(out, "\\n");
break;
case '\r':
g_string_append(out, "\\r");
break;
default:
g_string_append_c(out, *p);
break;
}
}
return g_string_free(out, FALSE);
}
// Unescape message body: \\ -> \, \n -> newline, \r -> CR
static char*
_ff_unescape_message(const char* text)
{
if (!text)
return g_strdup("");
GString* out = g_string_sized_new(strlen(text));
for (const char* p = text; *p; p++) {
if (*p == '\\' && *(p + 1)) {
p++;
switch (*p) {
case '\\':
g_string_append_c(out, '\\');
break;
case 'n':
g_string_append_c(out, '\n');
break;
case 'r':
g_string_append_c(out, '\r');
break;
default:
g_string_append_c(out, '\\');
g_string_append_c(out, *p);
break;
}
} else {
g_string_append_c(out, *p);
}
}
return g_string_free(out, FALSE);
}
// Escape metadata value (stanza_id, archive_id, replace_id):
// these come from remote servers and may contain |, ], \, newlines.
static char*
_ff_escape_meta_value(const char* val)
{
if (!val || strlen(val) == 0)
return NULL;
GString* out = g_string_sized_new(strlen(val));
jabber.developer marked this conversation as resolved Outdated

the logic seemingly mirrors the one from _ff_state_build, creating repetition. I suggest extracting it to a different method to keep things DRYer and more maintainable

the logic seemingly mirrors the one from `_ff_state_build`, creating repetition. I suggest extracting it to a different method to keep things DRYer and more maintainable

Corrected

Corrected
for (const char* p = val; *p; p++) {
switch (*p) {
case '|':
g_string_append(out, "\\|");
break;
case ']':
g_string_append(out, "\\]");
break;
case '\\':
g_string_append(out, "\\\\");
break;
case '\n':
g_string_append(out, "\\n");
break;
case '\r':
g_string_append(out, "\\r");
break;
default:
g_string_append_c(out, *p);
break;
}
}
return g_string_free(out, FALSE);
}
// Unescape metadata value
static char*
_ff_unescape_meta_value(const char* val)
{
if (!val)
return NULL;
GString* out = g_string_sized_new(strlen(val));
for (const char* p = val; *p; p++) {
if (*p == '\\' && *(p + 1)) {
p++;
switch (*p) {
case '|':
g_string_append_c(out, '|');
break;
case ']':
g_string_append_c(out, ']');
break;
case '\\':
g_string_append_c(out, '\\');
break;
case 'n':
g_string_append_c(out, '\n');
break;
case 'r':
g_string_append_c(out, '\r');
break;
default:
g_string_append_c(out, '\\');
g_string_append_c(out, *p);
break;
}
} else {
g_string_append_c(out, *p);
}
}
return g_string_free(out, FALSE);
}
// --- Readline helper ---
//
// POSIX getline() for dynamic-length lines. Returns line without trailing
// newline, or NULL on EOF. Sets *truncated=TRUE if the line had no trailing
// newline at EOF (partial write detection).
static char*
_ff_readline(FILE* fp, gboolean* truncated)
{
char* line = NULL;
size_t cap = 0;
ssize_t nread = getline(&line, &cap, fp);
if (nread == -1) {
free(line);
return NULL;
}
// Guard against pathological lines that could exhaust memory.
// getline() already allocated, so we free and skip if too long.
if (nread > FF_MAX_LINE_LEN) {
log_error("flatfile: line too long (%zd bytes), skipping", nread);
// Check newline termination before freeing
gboolean had_newline = (nread > 0 && line[nread - 1] == '\n');
free(line);
// Skip to next newline if the overlength line wasn't newline-terminated
if (!had_newline) {
int ch;
while ((ch = fgetc(fp)) != EOF && ch != '\n') {}
}
if (truncated)
*truncated = FALSE;
// Return empty string so caller's loop continues (parse will reject it)
return g_strdup("");
}
if (truncated)
*truncated = FALSE;
if (nread > 0 && line[nread - 1] == '\n') {
line[--nread] = '\0';
} else if (feof(fp)) {
// Line without trailing newline at EOF — likely a partial write
if (truncated)
*truncated = TRUE;
}
return line;
}
// --- Line format --- // --- Line format ---
// //
// Format: {ISO8601} [{type}|{enc}|id:{stanza_id}|aid:{archive_id}|corrects:{replace_id}] {from_jid}/{resource}: {message} // Format: {ISO8601} [{type}|{enc}|id:{stanza_id}|aid:{archive_id}|corrects:{replace_id}] {from_jid}/{resource}: {message}
// //
// Metadata fields after type|enc are optional. // Message text is escaped: \\ -> \\\\, newline -> \\n, CR -> \\r
// Metadata values are escaped: |, ], \\, newline, CR
// Lines starting with '#' are comments. // Lines starting with '#' are comments.
// Empty lines are skipped. // Empty lines are skipped.
@@ -175,41 +405,162 @@ _ff_write_line(FILE* fp, const char* timestamp, const char* type, const char* en
const char* stanza_id, const char* archive_id, const char* replace_id, const char* stanza_id, const char* archive_id, const char* replace_id,
const char* from_jid, const char* from_resource, const char* message_text) const char* from_jid, const char* from_resource, const char* message_text)
{ {
// Escape metadata values from remote peers
jabber.developer marked this conversation as resolved Outdated

note to check: is it case sensitive/get forced lowercase when set by user?

note to check: is it case sensitive/get forced lowercase when set by user?

Checked cmd_funcs.c:6676:


        if (g_strcmp0(arg, "on") == 0) {
            cons_show("Logging enabled.");
            prefs_set_string(PREF_DBLOG, arg);
            prefs_set_boolean(PREF_CHLOG, TRUE);
            prefs_set_boolean(PREF_HISTORY, TRUE);
        } else if (g_strcmp0(arg, "off") == 0) {
            cons_show("Logging disabled.");
            prefs_set_string(PREF_DBLOG, arg);
            prefs_set_boolean(PREF_CHLOG, FALSE);
            prefs_set_boolean(PREF_HISTORY, FALSE);
        }

resolved

Checked cmd_funcs.c:6676: ```c if (g_strcmp0(arg, "on") == 0) { cons_show("Logging enabled."); prefs_set_string(PREF_DBLOG, arg); prefs_set_boolean(PREF_CHLOG, TRUE); prefs_set_boolean(PREF_HISTORY, TRUE); } else if (g_strcmp0(arg, "off") == 0) { cons_show("Logging disabled."); prefs_set_string(PREF_DBLOG, arg); prefs_set_boolean(PREF_CHLOG, FALSE); prefs_set_boolean(PREF_HISTORY, FALSE); } ``` resolved
auto_gchar gchar* safe_sid = _ff_escape_meta_value(stanza_id);
auto_gchar gchar* safe_aid = _ff_escape_meta_value(archive_id);
auto_gchar gchar* safe_rid = _ff_escape_meta_value(replace_id);
// Build metadata section: [type|enc|id:...|aid:...|corrects:...] // Build metadata section: [type|enc|id:...|aid:...|corrects:...]
GString* meta = g_string_new("["); GString* meta = g_string_new("[");
g_string_append(meta, type ? type : "chat"); g_string_append(meta, type ? type : "chat");
g_string_append_c(meta, '|'); g_string_append_c(meta, '|');
g_string_append(meta, enc ? enc : "none"); g_string_append(meta, enc ? enc : "none");
if (stanza_id && strlen(stanza_id) > 0) { if (safe_sid) {
g_string_append_printf(meta, "|id:%s", stanza_id); g_string_append_printf(meta, "|id:%s", safe_sid);
} }
if (archive_id && strlen(archive_id) > 0) { if (safe_aid) {
g_string_append_printf(meta, "|aid:%s", archive_id); g_string_append_printf(meta, "|aid:%s", safe_aid);
} }
if (replace_id && strlen(replace_id) > 0) { if (safe_rid) {
g_string_append_printf(meta, "|corrects:%s", replace_id); g_string_append_printf(meta, "|corrects:%s", safe_rid);
} }
g_string_append_c(meta, ']'); g_string_append_c(meta, ']');
jabber.developer marked this conversation as resolved Outdated

nit: for readability, maybe we can do something like that?

gboolean is_incoming = myjid && myjid->barejid && g_strcmp0(from_barejid, myjid->barejid) == 0;
contact = is_incoming ? to_barejid : from_barejid;
nit: for readability, maybe we can do something like that? ```c gboolean is_incoming = myjid && myjid->barejid && g_strcmp0(from_barejid, myjid->barejid) == 0; contact = is_incoming ? to_barejid : from_barejid; ```

Corrected

Corrected
// Build sender // Build sender — escape ": " in the resource part to prevent
// the parser from splitting at the wrong point.
// JID bare parts (user@domain) never contain ": " but XMPP resources
// can contain arbitrary UTF-8 including ": ".
GString* sender = g_string_new(from_jid ? from_jid : "unknown"); GString* sender = g_string_new(from_jid ? from_jid : "unknown");
if (from_resource && strlen(from_resource) > 0) { if (from_resource && strlen(from_resource) > 0) {
g_string_append_printf(sender, "/%s", from_resource); // Escape backslash and colon-space in resource
GString* safe_res = g_string_sized_new(strlen(from_resource));
for (const char* p = from_resource; *p; p++) {
if (*p == '\\') {
g_string_append(safe_res, "\\\\");
} else if (*p == ':' && *(p + 1) == ' ') {
g_string_append(safe_res, "\\: ");
p++; // skip the space too
} else {
g_string_append_c(safe_res, *p);
}
}
g_string_append_printf(sender, "/%s", safe_res->str);
g_string_free(safe_res, TRUE);
} }
fprintf(fp, "%s %s %s: %s\n", // Escape message body to prevent log injection
jabber.developer marked this conversation as resolved Outdated

O_APPEND won't execute since O_EXCL would throw an error if file already exists.

`O_APPEND` won't execute since `O_EXCL` would throw an error if file already exists.

Corrected

Corrected
timestamp, char* safe_msg = _ff_escape_message(message_text);
meta->str,
sender->str,
message_text ? message_text : "");
// Build complete line and write atomically with a single write()
GString* full_line = g_string_new(NULL);
g_string_printf(full_line, "%s %s %s: %s\n",
timestamp, meta->str, sender->str, safe_msg);
size_t to_write = full_line->len;
ssize_t written = fwrite(full_line->str, 1, to_write, fp);
if (written != (ssize_t)to_write) {
log_error("flatfile: partial write (%zd/%zu)", written, to_write);
}
g_string_free(full_line, TRUE);
g_free(safe_msg);
g_string_free(meta, TRUE); g_string_free(meta, TRUE);
g_string_free(sender, TRUE); g_string_free(sender, TRUE);
} }
// --- Line parser helpers ---
// Find the first occurrence of 'ch' that is not preceded by an unescaped backslash.
// Returns pointer to 'ch' in 'str', or NULL if not found.
static const char*
_ff_find_unescaped_char(const char* str, char ch)
{
if (!str)
return NULL;
for (const char* p = str; *p; p++) {
if (*p == '\\' && *(p + 1)) {
p++; // skip escaped character
continue;
}
if (*p == ch)
return p;
}
return NULL;
}
// Split metadata content on unescaped '|'. Returns a NULL-terminated array.
// Caller must g_strfreev() the result.
static char**
_ff_split_meta(const char* meta)
{
GPtrArray* arr = g_ptr_array_new();
const char* start = meta;
for (const char* p = meta;; p++) {
if (*p == '\\' && *(p + 1)) {
p++; // skip escaped char
continue;
}
if (*p == '|' || *p == '\0') {
g_ptr_array_add(arr, g_strndup(start, p - start));
if (*p == '\0')
break;
start = p + 1;
}
}
g_ptr_array_add(arr, NULL);
return (char**)g_ptr_array_free(arr, FALSE);
}
// Find first unescaped ": " (colon-space) in a string.
// Escaped form is "\: " — a backslash before the colon.
static const char*
_ff_find_unescaped_colonspace(const char* str)
{
if (!str)
return NULL;
for (const char* p = str; *p; p++) {
if (*p == '\\' && *(p + 1)) {
p++; // skip escaped character
continue;
}
if (*p == ':' && *(p + 1) == ' ') {
return p;
}
}
return NULL;
}
// Unescape a sender resource: "\\" -> '\', "\: " -> ": "
static char*
_ff_unescape_sender_resource(const char* res)
{
if (!res)
return NULL;
GString* out = g_string_sized_new(strlen(res));
for (const char* p = res; *p; p++) {
if (*p == '\\' && *(p + 1)) {
p++;
if (*p == '\\') {
g_string_append_c(out, '\\');
} else if (*p == ':' && *(p + 1) == ' ') {
g_string_append(out, ": ");
p++; // skip the space
} else {
// Unknown escape — preserve literally
g_string_append_c(out, '\\');
g_string_append_c(out, *p);
}
} else {
g_string_append_c(out, *p);
}
}
return g_string_free(out, FALSE);
}
// --- Line parser --- // --- Line parser ---
typedef struct { typedef struct
{
char* timestamp_str; char* timestamp_str;
GDateTime* timestamp; GDateTime* timestamp;
char* type; char* type;
@@ -225,9 +576,11 @@ typedef struct {
static void static void
_ff_parsed_line_free(flatfile_parsed_line_t* pl) _ff_parsed_line_free(flatfile_parsed_line_t* pl)
{ {
if (!pl) return; if (!pl)
return;
g_free(pl->timestamp_str); g_free(pl->timestamp_str);
if (pl->timestamp) g_date_time_unref(pl->timestamp); if (pl->timestamp)
g_date_time_unref(pl->timestamp);
g_free(pl->type); g_free(pl->type);
g_free(pl->enc); g_free(pl->enc);
g_free(pl->stanza_id); g_free(pl->stanza_id);
@@ -272,7 +625,8 @@ _ff_parse_line(const char* line)
g_free(work); g_free(work);
work = converted; work = converted;
} else { } else {
if (err) g_error_free(err); if (err)
g_error_free(err);
g_free(work); g_free(work);
return NULL; return NULL;
} }
@@ -290,12 +644,13 @@ _ff_parse_line(const char* line)
result->timestamp_str = g_strndup(work, first_space - work); result->timestamp_str = g_strndup(work, first_space - work);
// Parse metadata section [...] // Parse metadata section [...]
char* bracket_end = strchr(bracket_start, ']'); // Use escape-aware bracket finder to handle escaped ']' in values
const char* bracket_end = _ff_find_unescaped_char(bracket_start + 1, ']');
if (bracket_end) { if (bracket_end) {
char* meta_content = g_strndup(bracket_start + 1, bracket_end - bracket_start - 1); char* meta_content = g_strndup(bracket_start + 1, bracket_end - bracket_start - 1);
// Split by '|' // Split by unescaped '|'
char** parts = g_strsplit(meta_content, "|", -1); char** parts = _ff_split_meta(meta_content);
if (parts) { if (parts) {
int i = 0; int i = 0;
for (; parts[i]; i++) { for (; parts[i]; i++) {
@@ -304,11 +659,11 @@ _ff_parse_line(const char* line)
} else if (i == 1) { } else if (i == 1) {
result->enc = g_strdup(parts[i]); result->enc = g_strdup(parts[i]);
} else if (g_str_has_prefix(parts[i], "id:")) { } else if (g_str_has_prefix(parts[i], "id:")) {
result->stanza_id = g_strdup(parts[i] + 3); result->stanza_id = _ff_unescape_meta_value(parts[i] + 3);
} else if (g_str_has_prefix(parts[i], "aid:")) { } else if (g_str_has_prefix(parts[i], "aid:")) {
result->archive_id = g_strdup(parts[i] + 4); result->archive_id = _ff_unescape_meta_value(parts[i] + 4);
} else if (g_str_has_prefix(parts[i], "corrects:")) { } else if (g_str_has_prefix(parts[i], "corrects:")) {
result->replace_id = g_strdup(parts[i] + 9); result->replace_id = _ff_unescape_meta_value(parts[i] + 9);
} }
} }
g_strfreev(parts); g_strfreev(parts);
jabber.developer marked this conversation as resolved
Review

we should be careful with LMC, since it can be disabled by user, so this part needs further review and discussion.

we should be careful with LMC, since it can be disabled by user, so this part needs further review and discussion.

Corrected

Corrected
@@ -316,29 +671,31 @@ _ff_parse_line(const char* line)
g_free(meta_content); g_free(meta_content);
// Parse sender: message after '] ' // Parse sender: message after '] '
char* after_meta = bracket_end + 1; const char* after_meta = bracket_end + 1;
if (*after_meta == ' ') after_meta++; if (*after_meta == ' ')
after_meta++;
// Find first ': ' which separates sender from message // Find first *unescaped* ': ' which separates sender from message.
char* colon = strstr(after_meta, ": "); // Resources may contain escaped \: sequences.
const char* colon = _ff_find_unescaped_colonspace(after_meta);
if (colon) { if (colon) {
char* sender = g_strndup(after_meta, colon - after_meta); char* raw_sender = g_strndup(after_meta, colon - after_meta);
// Split sender into jid/resource // Split sender into jid/resource, then unescape resource
char* slash = strchr(sender, '/'); char* slash = strchr(raw_sender, '/');
if (slash) { if (slash) {
result->from_jid = g_strndup(sender, slash - sender); result->from_jid = g_strndup(raw_sender, slash - raw_sender);
result->from_resource = g_strdup(slash + 1); result->from_resource = _ff_unescape_sender_resource(slash + 1);
} else { } else {
result->from_jid = g_strdup(sender); result->from_jid = g_strdup(raw_sender);
} }
g_free(sender); g_free(raw_sender);
result->message = g_strdup(colon + 2); result->message = _ff_unescape_message(colon + 2);
} else { } else {
// No ': ' found, treat entire rest as message with unknown sender // No ': ' found, treat entire rest as message with unknown sender
result->from_jid = g_strdup("unknown"); result->from_jid = g_strdup("unknown");
result->message = g_strdup(after_meta); result->message = _ff_unescape_message(after_meta);
} }
} else { } else {
// No closing bracket — malformed metadata // No closing bracket — malformed metadata
@@ -370,10 +727,10 @@ _ff_parse_line(const char* line)
result->from_jid = g_strdup(sender); result->from_jid = g_strdup(sender);
} }
g_free(sender); g_free(sender);
result->message = g_strdup(colon + 2); result->message = _ff_unescape_message(colon + 2);
} else { } else {
result->from_jid = g_strdup("unknown"); result->from_jid = g_strdup("unknown");
result->message = g_strdup(rest); result->message = _ff_unescape_message(rest);
} }
} else { } else {
// No space at all — can't parse // No space at all — can't parse
@@ -385,15 +742,17 @@ _ff_parse_line(const char* line)
// Parse timestamp // Parse timestamp
result->timestamp = g_date_time_new_from_iso8601(result->timestamp_str, NULL); result->timestamp = g_date_time_new_from_iso8601(result->timestamp_str, NULL);
if (!result->timestamp) { if (!result->timestamp) {
log_warning("flatfile: unparseable timestamp: %s", result->timestamp_str); log_warning("flatfile: unparsable timestamp: %s", result->timestamp_str);
_ff_parsed_line_free(result); _ff_parsed_line_free(result);
g_free(work); g_free(work);
return NULL; return NULL;
} }
// Default type/enc if missing // Default type/enc if missing
if (!result->type) result->type = g_strdup("chat"); if (!result->type)
if (!result->enc) result->enc = g_strdup("none"); result->type = g_strdup("chat");
if (!result->enc)
result->enc = g_strdup("none");
g_free(work); g_free(work);
return result; return result;
@@ -463,19 +822,36 @@ _ff_add_message(const char* type, const char* stanza_id, const char* archive_id,
return; return;
} }
// Check if file is new (needs header) // Open the file with O_NOFOLLOW to prevent symlink attacks,
gboolean is_new = !g_file_test(log_path, G_FILE_TEST_EXISTS); // and O_APPEND for safe concurrent appends.
// Try O_CREAT|O_EXCL first to detect new files atomically (no TOCTOU).
int fd = open(log_path, O_WRONLY | O_APPEND | O_CREAT | O_EXCL | O_NOFOLLOW,
S_IRUSR | S_IWUSR);
gboolean is_new = (fd >= 0);
if (fd < 0) {
if (errno == EEXIST) {
// File already exists — open for append
fd = open(log_path, O_WRONLY | O_APPEND | O_NOFOLLOW);
}
if (fd < 0) {
// ELOOP (symlink) or other error
log_error("flatfile: could not open %s for writing (errno=%d: %s)",
log_path, errno, strerror(errno));
g_free(effective_msg);
return;
}
}
FILE* fp = fopen(log_path, "a"); FILE* fp = fdopen(fd, "a");
if (!fp) { if (!fp) {
log_error("flatfile: could not open %s for writing (errno=%d)", log_path, errno); log_error("flatfile: fdopen failed for %s (errno=%d)", log_path, errno);
close(fd);
g_free(effective_msg); g_free(effective_msg);
return; return;
} }
if (is_new) { if (is_new) {
fprintf(fp, "%s", FLATFILE_HEADER); fprintf(fp, "%s", FLATFILE_HEADER);
g_chmod(log_path, S_IRUSR | S_IWUSR);
} }
_ff_write_line(fp, date_fmt, type, _ff_get_message_enc_str(enc), _ff_write_line(fp, date_fmt, type, _ff_get_message_enc_str(enc),
@@ -524,7 +900,8 @@ _flatfile_close(void)
static void static void
_flatfile_add_incoming(ProfMessage* message) _flatfile_add_incoming(ProfMessage* message)
{ {
if (!message || !message->from_jid) return; if (!message || !message->from_jid)
return;
const Jid* to_jid = message->to_jid ? message->to_jid : connection_get_jid(); const Jid* to_jid = message->to_jid ? message->to_jid : connection_get_jid();
const char* type = _ff_get_message_type_str(message->type); const char* type = _ff_get_message_type_str(message->type);
@@ -577,7 +954,8 @@ _ff_list_log_files(const char* contact_barejid)
} }
GDir* dir = g_dir_open(contact_dir, 0, NULL); GDir* dir = g_dir_open(contact_dir, 0, NULL);
if (!dir) return NULL; if (!dir)
return NULL;
GSList* files = NULL; GSList* files = NULL;
const gchar* fname; const gchar* fname;
@@ -601,7 +979,8 @@ _ff_read_file_messages(const char* filepath, const char* contact_barejid,
GSList* messages = NULL; GSList* messages = NULL;
FILE* fp = fopen(filepath, "r"); FILE* fp = fopen(filepath, "r");
if (!fp) return NULL; if (!fp)
return NULL;
// BOM detection // BOM detection
int c1 = fgetc(fp); int c1 = fgetc(fp);
@@ -612,14 +991,19 @@ _ff_read_file_messages(const char* filepath, const char* contact_barejid,
fseek(fp, 0, SEEK_SET); fseek(fp, 0, SEEK_SET);
} }
char buf[8192]; char* buf = NULL;
while (fgets(buf, sizeof(buf), fp)) { gboolean truncated = FALSE;
// Strip trailing newline while ((buf = _ff_readline(fp, &truncated)) != NULL) {
gsize len = strlen(buf); if (truncated) {
if (len > 0 && buf[len - 1] == '\n') buf[--len] = '\0'; log_warning("flatfile: truncated line at EOF in %s, skipping", filepath);
free(buf);
break;
}
flatfile_parsed_line_t* pl = _ff_parse_line(buf); flatfile_parsed_line_t* pl = _ff_parse_line(buf);
if (!pl) continue; free(buf);
if (!pl)
continue;
// Check timestamp bounds // Check timestamp bounds
if (start_time) { if (start_time) {
@@ -648,8 +1032,7 @@ _ff_read_file_messages(const char* filepath, const char* contact_barejid,
// Check JID filter: message must involve contact and our JID // Check JID filter: message must involve contact and our JID
gboolean matches = FALSE; gboolean matches = FALSE;
if (myjid && myjid->barejid && contact_barejid) { if (myjid && myjid->barejid && contact_barejid) {
if ((g_strcmp0(pl->from_jid, contact_barejid) == 0) || if ((g_strcmp0(pl->from_jid, contact_barejid) == 0) || (g_strcmp0(pl->from_jid, myjid->barejid) == 0)) {
(g_strcmp0(pl->from_jid, myjid->barejid) == 0)) {
matches = TRUE; matches = TRUE;
} }
} else { } else {
@@ -691,16 +1074,25 @@ _ff_apply_lmc(GSList* parsed_lines, GSList** result)
if (pl->replace_id && strlen(pl->replace_id) > 0) { if (pl->replace_id && strlen(pl->replace_id) > 0) {
flatfile_parsed_line_t* original = g_hash_table_lookup(id_map, pl->replace_id); flatfile_parsed_line_t* original = g_hash_table_lookup(id_map, pl->replace_id);
if (original) { if (original) {
// Follow chain to the root original // Follow chain to the root original, with cycle/depth guard
flatfile_parsed_line_t* root = original; flatfile_parsed_line_t* root = original;
while (root->replace_id && strlen(root->replace_id) > 0) { GHashTable* visited = g_hash_table_new(g_direct_hash, g_direct_equal);
g_hash_table_insert(visited, root, root);
int depth = 0;
while (root->replace_id && strlen(root->replace_id) > 0 && depth < FF_MAX_LMC_DEPTH) {
flatfile_parsed_line_t* parent = g_hash_table_lookup(id_map, root->replace_id); flatfile_parsed_line_t* parent = g_hash_table_lookup(id_map, root->replace_id);
if (parent && parent != root) { if (parent && !g_hash_table_contains(visited, parent)) {
g_hash_table_insert(visited, parent, parent);
root = parent; root = parent;
depth++;
} else { } else {
break; break;
} }
} }
g_hash_table_destroy(visited);
if (depth >= FF_MAX_LMC_DEPTH) {
log_warning("flatfile: LMC correction chain too deep (>%d), ignoring", FF_MAX_LMC_DEPTH);
}
g_hash_table_insert(correction_map, root, pl); g_hash_table_insert(correction_map, root, pl);
g_hash_table_insert(corrections, pl, pl); g_hash_table_insert(corrections, pl, pl);
} }
@@ -797,7 +1189,8 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta
GSList* nth = g_slist_nth(pre_result, MESSAGES_TO_RETRIEVE); GSList* nth = g_slist_nth(pre_result, MESSAGES_TO_RETRIEVE);
if (nth) { if (nth) {
GSList* prev = g_slist_nth(pre_result, MESSAGES_TO_RETRIEVE - 1); GSList* prev = g_slist_nth(pre_result, MESSAGES_TO_RETRIEVE - 1);
if (prev) prev->next = NULL; if (prev)
prev->next = NULL;
g_slist_free_full(nth, (GDestroyNotify)message_free); g_slist_free_full(nth, (GDestroyNotify)message_free);
} }
} else { } else {
@@ -805,7 +1198,8 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta
guint skip = total - MESSAGES_TO_RETRIEVE; guint skip = total - MESSAGES_TO_RETRIEVE;
GSList* keep_start = g_slist_nth(pre_result, skip); GSList* keep_start = g_slist_nth(pre_result, skip);
GSList* prev = g_slist_nth(pre_result, skip - 1); GSList* prev = g_slist_nth(pre_result, skip - 1);
if (prev) prev->next = NULL; if (prev)
prev->next = NULL;
g_slist_free_full(pre_result, (GDestroyNotify)message_free); g_slist_free_full(pre_result, (GDestroyNotify)message_free);
pre_result = keep_start; pre_result = keep_start;
} }
@@ -826,13 +1220,15 @@ _flatfile_get_limits_info(const gchar* const contact_barejid, gboolean is_last)
ProfMessage* msg = message_init(); ProfMessage* msg = message_init();
if (!g_flatfile_account_jid || !contact_barejid) { if (!g_flatfile_account_jid || !contact_barejid) {
if (is_last) msg->timestamp = g_date_time_new_now_utc(); if (is_last)
msg->timestamp = g_date_time_new_now_utc();
return msg; return msg;
} }
GSList* files = _ff_list_log_files(contact_barejid); GSList* files = _ff_list_log_files(contact_barejid);
if (!files) { if (!files) {
if (is_last) msg->timestamp = g_date_time_new_now_utc(); if (is_last)
msg->timestamp = g_date_time_new_now_utc();
return msg; return msg;
} }
@@ -844,7 +1240,8 @@ _flatfile_get_limits_info(const gchar* const contact_barejid, gboolean is_last)
FILE* fp = fopen(target_file, "r"); FILE* fp = fopen(target_file, "r");
if (!fp) { if (!fp) {
g_slist_free_full(files, g_free); g_slist_free_full(files, g_free);
if (is_last) msg->timestamp = g_date_time_new_now_utc(); if (is_last)
msg->timestamp = g_date_time_new_now_utc();
return msg; return msg;
} }
@@ -856,25 +1253,25 @@ _flatfile_get_limits_info(const gchar* const contact_barejid, gboolean is_last)
fseek(fp, 0, SEEK_SET); fseek(fp, 0, SEEK_SET);
} }
char buf[8192]; char* buf = NULL;
flatfile_parsed_line_t* found = NULL; flatfile_parsed_line_t* found = NULL;
if (!is_last) { if (!is_last) {
// Find first valid line // Find first valid line
while (fgets(buf, sizeof(buf), fp)) { while ((buf = _ff_readline(fp, NULL)) != NULL) {
gsize len = strlen(buf);
if (len > 0 && buf[len - 1] == '\n') buf[--len] = '\0';
found = _ff_parse_line(buf); found = _ff_parse_line(buf);
if (found) break; free(buf);
if (found)
break;
} }
} else { } else {
// Find last valid line — read all and keep last // Find last valid line — read all and keep last
while (fgets(buf, sizeof(buf), fp)) { while ((buf = _ff_readline(fp, NULL)) != NULL) {
gsize len = strlen(buf);
if (len > 0 && buf[len - 1] == '\n') buf[--len] = '\0';
flatfile_parsed_line_t* pl = _ff_parse_line(buf); flatfile_parsed_line_t* pl = _ff_parse_line(buf);
free(buf);
if (pl) { if (pl) {
if (found) _ff_parsed_line_free(found); if (found)
_ff_parsed_line_free(found);
found = pl; found = pl;
} }
} }
@@ -952,7 +1349,8 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
const char* cdir_path = cd->data; const char* cdir_path = cd->data;
GDir* dir = g_dir_open(cdir_path, 0, NULL); GDir* dir = g_dir_open(cdir_path, 0, NULL);
if (!dir) continue; if (!dir)
continue;
GSList* log_files = NULL; GSList* log_files = NULL;
const gchar* fname; const gchar* fname;
@@ -988,7 +1386,8 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
} }
FILE* fp = fopen(filepath, "r"); FILE* fp = fopen(filepath, "r");
if (!fp) continue; if (!fp)
continue;
// BOM check (4l) // BOM check (4l)
int c1 = fgetc(fp); int c1 = fgetc(fp);
@@ -1005,7 +1404,7 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
fseek(fp, 0, SEEK_SET); fseek(fp, 0, SEEK_SET);
} }
char buf[8192]; char* buf = NULL;
int lineno = 0; int lineno = 0;
GDateTime* prev_ts = NULL; GDateTime* prev_ts = NULL;
GDateTime* first_ts = NULL; GDateTime* first_ts = NULL;
@@ -1013,10 +1412,9 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
gboolean has_crlf = FALSE; gboolean has_crlf = FALSE;
gboolean is_empty = TRUE; gboolean is_empty = TRUE;
while (fgets(buf, sizeof(buf), fp)) { while ((buf = _ff_readline(fp, NULL)) != NULL) {
lineno++; lineno++;
gsize len = strlen(buf); gsize len = strlen(buf);
if (len > 0 && buf[len - 1] == '\n') buf[--len] = '\0';
// CRLF check (4m) // CRLF check (4m)
if (len > 0 && buf[len - 1] == '\r') { if (len > 0 && buf[len - 1] == '\r') {
@@ -1025,7 +1423,10 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
} }
// Skip empty lines and comments // Skip empty lines and comments
if (len == 0 || buf[0] == '#') continue; if (len == 0 || buf[0] == '#') {
free(buf);
continue;
}
is_empty = FALSE; is_empty = FALSE;
// UTF-8 validation (4k/4n) // UTF-8 validation (4k/4n)
@@ -1037,6 +1438,7 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
issue->line = lineno; issue->line = lineno;
issue->message = g_strdup_printf("Invalid UTF-8 at byte offset %ld", (long)(end - buf)); issue->message = g_strdup_printf("Invalid UTF-8 at byte offset %ld", (long)(end - buf));
issues = g_slist_append(issues, issue); issues = g_slist_append(issues, issue);
free(buf);
continue; continue;
} }
@@ -1061,13 +1463,18 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
issue->level = INTEGRITY_ERROR; issue->level = INTEGRITY_ERROR;
issue->file = g_strdup(basename); issue->file = g_strdup(basename);
issue->line = lineno; issue->line = lineno;
issue->message = g_strdup("Unparseable line"); issue->message = g_strdup("Unparsable line");
issues = g_slist_append(issues, issue); issues = g_slist_append(issues, issue);
free(buf);
continue; continue;
} }
if (!first_ts) first_ts = g_date_time_ref(pl->timestamp); free(buf); // done with raw line
if (last_ts) g_date_time_unref(last_ts);
if (!first_ts)
first_ts = g_date_time_ref(pl->timestamp);
if (last_ts)
g_date_time_unref(last_ts);
last_ts = g_date_time_ref(pl->timestamp); last_ts = g_date_time_ref(pl->timestamp);
// Timestamp order within file (4c) // Timestamp order within file (4c)
@@ -1081,7 +1488,8 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
issue->message = g_strdup_printf("Timestamp out of order (%s after %s)", ts_cur, ts_prev); issue->message = g_strdup_printf("Timestamp out of order (%s after %s)", ts_cur, ts_prev);
issues = g_slist_append(issues, issue); issues = g_slist_append(issues, issue);
} }
if (prev_ts) g_date_time_unref(prev_ts); if (prev_ts)
g_date_time_unref(prev_ts);
prev_ts = g_date_time_ref(pl->timestamp); prev_ts = g_date_time_ref(pl->timestamp);
// Duplicate stanza-id / archive-id (4f) // Duplicate stanza-id / archive-id (4f)
@@ -1155,12 +1563,16 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
} }
} }
if (prev_file_last_ts) g_date_time_unref(prev_file_last_ts); if (prev_file_last_ts)
g_date_time_unref(prev_file_last_ts);
prev_file_last_ts = last_ts ? g_date_time_ref(last_ts) : NULL; prev_file_last_ts = last_ts ? g_date_time_ref(last_ts) : NULL;
if (prev_ts) g_date_time_unref(prev_ts); if (prev_ts)
if (first_ts) g_date_time_unref(first_ts); g_date_time_unref(prev_ts);
if (last_ts) g_date_time_unref(last_ts); if (first_ts)
g_date_time_unref(first_ts);
if (last_ts)
g_date_time_unref(last_ts);
} }
// Second pass: check LMC references across all files (4g) // Second pass: check LMC references across all files (4g)
@@ -1170,7 +1582,8 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
basename_lmc = basename_lmc ? basename_lmc + 1 : filepath; basename_lmc = basename_lmc ? basename_lmc + 1 : filepath;
FILE* fp = fopen(filepath, "r"); FILE* fp = fopen(filepath, "r");
if (!fp) continue; if (!fp)
continue;
// Skip BOM // Skip BOM
int b1 = fgetc(fp); int b1 = fgetc(fp);
@@ -1180,17 +1593,22 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
fseek(fp, 0, SEEK_SET); fseek(fp, 0, SEEK_SET);
} }
char buf[8192]; char* buf = NULL;
int lineno = 0; int lineno = 0;
while (fgets(buf, sizeof(buf), fp)) { while ((buf = _ff_readline(fp, NULL)) != NULL) {
lineno++; lineno++;
gsize len = strlen(buf); gsize len = strlen(buf);
if (len > 0 && buf[len - 1] == '\n') buf[--len] = '\0'; if (len > 0 && buf[len - 1] == '\r')
if (len > 0 && buf[len - 1] == '\r') buf[--len] = '\0'; buf[--len] = '\0';
if (len == 0 || buf[0] == '#') continue; if (len == 0 || buf[0] == '#') {
free(buf);
continue;
}
flatfile_parsed_line_t* pl = _ff_parse_line(buf); flatfile_parsed_line_t* pl = _ff_parse_line(buf);
if (!pl) continue; free(buf);
if (!pl)
continue;
if (pl->replace_id && strlen(pl->replace_id) > 0) { if (pl->replace_id && strlen(pl->replace_id) > 0) {
if (!g_hash_table_contains(all_stanza_ids, pl->replace_id)) { if (!g_hash_table_contains(all_stanza_ids, pl->replace_id)) {
@@ -1207,7 +1625,8 @@ _flatfile_verify_integrity(const gchar* const contact_barejid)
fclose(fp); fclose(fp);
} }
if (prev_file_last_ts) g_date_time_unref(prev_file_last_ts); if (prev_file_last_ts)
g_date_time_unref(prev_file_last_ts);
g_hash_table_destroy(seen_ids); g_hash_table_destroy(seen_ids);
g_hash_table_destroy(all_stanza_ids); g_hash_table_destroy(all_stanza_ids);
g_slist_free_full(log_files, g_free); g_slist_free_full(log_files, g_free);