mod_mail_trash: Pass full directory path to maildir_indicate_expunged.

Previously, we were only passing the base directory name to
a function that expects the full path to the maildir's curdir.
Construct and pass the correct path, and consolidate and add
checks to make sure clearly invalid maildir paths are not being
used.

LBBS-25 #close

bbs/backtrace.c

@@ -132,8 +132,9 @@ static void process_section(bfd *bfdobj, asection *section, void *obj)
 		/* file can possibly be null even with a success result from bfd_find_nearest_line */
 		file = file ? file : "";
 		fn = strrchr(file, '/');
-#define FMT_INLINED     "[%14s] %-17s %24s:%-5u %s()"
-#define FMT_NOT_INLINED "[%14p] %-17s %24s:%-5u %s()"
+		/* The second argument is the .so and the third is the .c, so fmt length of arg 2 should be one more than of arg 3 */
+#define FMT_INLINED     "[%14s] %-25s %24s:%-5u %s()"
+#define FMT_NOT_INLINED "[%14p] %-25s %24s:%-5u %s()"
 
 		snprintf(data->msg, BT_MSG_BUFF_LEN, inlined ? FMT_INLINED : FMT_NOT_INLINED,
 			inlined ? "inlined" : (char *)(uintptr_t) data->pc,

include/mod_mail.h

@@ -269,14 +269,21 @@ unsigned long mailbox_quota_remaining(struct mailbox *mbox);
 
 /*!
  * \brief Get the maildir of a mailbox
- * \param mbox Mailbox. If NULL, the top-level maildir path will be returned.
+ * \param mbox Mailbox. If NULL, the top-level (root) maildir path will be returned.
  * \return Full path to home maildir of mailbox.
  */
 const char *mailbox_maildir(struct mailbox *mbox) __attribute__((returns_nonnull));
 
 /*! \brief Get the string length of a mailbox's maildir */
 size_t mailbox_maildir_len(struct mailbox *mbox);
 
+/*!
+ * \brief Validate that a maildir path is valid
+ * \param maildir Full directory path, which will be validated to ensure it is strictly a subdirectory of mailbox_maildir(NULL)
+ * \retval -1 if invalid, 0 if valid
+ */
+int mailbox_maildir_validate(const char *maildir);
+
 /*!
  * \brief Get the mailbox ID of a mailbox for personal mailboxes (same as the user ID of the user that owns this mailbox)
  * \param mbox Mailbox. Must not be NULL.

modules/mod_mail.c

@@ -225,12 +225,15 @@ void mailbox_dispatch_event(struct mailbox_event *event)
 	event->id = ++next_eventid;
 	bbs_mutex_unlock(&eventidlock);
 
-	bbs_debug(6, "Dispatching mailbox event '%s'\n", mailbox_event_type_name(event->type));
-
 	/* Sanity checks */
 	bbs_assert(!event->uids || event->numuids > 0); /* If we provided UIDs, then we must specify how many */
 	bbs_assert(!event->maildir || event->maildir[0] == '/'); /* This is supposed to be a full path, not relative */
 
+	bbs_debug(6, "Dispatching mailbox event '%s' (maildir: %s)\n", mailbox_event_type_name(event->type), S_IF(event->maildir));
+	if (!strlen_zero(event->maildir)) {
+		mailbox_maildir_validate(event->maildir);
+	}
+
 	/* At this point in time, we have an event,
 	 * with all the information that was "free" for the caller to provide.
 	 * e.g. If the caller already knew the MODSEQ, it should be filled in,
@@ -890,11 +893,35 @@ const char *mailbox_maildir(struct mailbox *mbox)
 	return mbox->maildir;
 }
 
+#define CHECK_MAILDIR_LEN_INTEGRITY
+
 size_t mailbox_maildir_len(struct mailbox *mbox)
 {
+#ifdef CHECK_MAILDIR_LEN_INTEGRITY
+	if (strlen(mbox->maildir) != mbox->maildirlen) {
+		bbs_warning("maildir length mismatch: %lu != %lu\n", strlen(mbox->maildir), mbox->maildirlen);
+		bbs_soft_assert(0);
+	}
+#endif
 	return mbox->maildirlen;
 }
 
+int mailbox_maildir_validate(const char *maildir)
+{
+	size_t dirlen = maildir ? strlen(maildir) : 0;
+	size_t rootmaildirlen = strlen(mailbox_maildir(NULL));
+
+	if (!dirlen) {
+		return 0;
+	}
+	if (dirlen <= rootmaildirlen) {
+		bbs_warning("maildir (%s) len (%lu) <= maildir root (%s) len (%lu)?\n", maildir, dirlen, mailbox_maildir(NULL), rootmaildirlen);
+		bbs_soft_assert(0);
+		return -1;
+	}
+	return 0;
+}
+
 int mailbox_id(struct mailbox *mbox)
 {
 	return (int) mbox->id;
@@ -1015,6 +1042,10 @@ unsigned int mailbox_get_next_uid(struct mailbox *mbox, struct bbs_node *node, c
 		}
 	}
 
+	/* This should be a subdirectory of the root maildir,
+	 * as there shouldn't be a UID file in that directory. */
+	mailbox_maildir_validate(directory);
+
 	/* A single file that stores the UIDVALIDITY and UIDNEXT for this folder.
 	 * We can't use a single file since the UIDNEXT for each directory has to be unique.
 	 * We probably wouldn't want to use the same UIDVALIDITY globally either for the entire mailbox,
@@ -1164,6 +1195,8 @@ static unsigned long __maildir_modseq(struct mailbox *mbox, const char *director
 
 	UNUSED(mbox); /* Not currently used, but could be useful for future caching strategies? */
 
+	bbs_soft_assert(strchr(directory, '/') != NULL);
+
 	/* Use a separate file from .uidvalidity for simplicity and ease of parsing, since this file is going to get used a lot more than the uidvalidity file
 	 * Also, since this file may be very large, since it needs to permanently store the MODSEQ of every single expunged message, forever.
 	 * For this reason, and for ease and speed of modifying the file in place, this is also a binary file, NOT a text file. */
@@ -1176,7 +1209,7 @@ static unsigned long __maildir_modseq(struct mailbox *mbox, const char *director
 		const char *modseq;
 		/* Order of traversal does not matter, so use opendir instead of scandir for efficiency. */
 		if (!(dir = opendir(directory))) {
-			bbs_error("Error opening directory - %s: %s\n", directory, strerror(errno));
+			bbs_error("Error opening directory '%s': %s\n", directory, strerror(errno));
 			return 0;
 		}
 
@@ -1346,6 +1379,7 @@ unsigned long maildir_indicate_expunged(enum mailbox_event_type type, struct bbs
 	mailbox_uid_lock(mbox);
 	maxmodseq = __maildir_modseq(mbox, directory, 1); /* Must be atomic */
 
+	bbs_soft_assert(strstr(directory, "/cur") != NULL); /* directory should be the curdir for this maildir, not the maildir itself */
 	snprintf(modseqfile, sizeof(modseqfile), "%s/../.modseqs", directory);
 	fp = fopen(modseqfile, "ab");
 	if (!fp) {

modules/mod_mail_events.c

@@ -55,8 +55,10 @@ static int generate_imap_uri(struct mailbox *mbox, const char *maildir, char *re
 	const char *mboxname = "";
 	size_t dirlen = maildir ? strlen(maildir) : 0;
 
+	mailbox_maildir_validate(maildir);
+	/* This check is on mbox (mailbox_maildir_validate just takes a path): */
 	if (dirlen && dirlen < mailbox_maildir_len(mbox)) {
-		bbs_warning("maildir len (%lu) is less than maildir root len (%lu)?\n", dirlen, mailbox_maildir_len(mbox));
+		bbs_warning("maildir (%s) len (%lu) is less than maildir root (%s) len (%lu)?\n", maildir, dirlen, mailbox_maildir(NULL), mailbox_maildir_len(mbox));
 		return -1;
 	}
 

modules/mod_mail_trash.c

@@ -28,6 +28,9 @@
 
 #include "include/mod_mail.h"
 
+/* Purely for testing/debugging. Always delete one message in the Trash when the trash reaper runs. */
+/* #define FORCE_DELETE_ONE_IMMEDIATELY */
+
 static unsigned int trashdays = 7;
 
 static pthread_t trash_thread = 0;
@@ -63,7 +66,11 @@ static int on_mailbox_trash(const char *dir_name, const char *filename, int seqn
 	tstamp = st.st_ctime;
 	elapsed = now - tstamp;
 	bbs_debug(7, "Encountered in trash: %s (%" TIME_T_FMT " s ago)\n", fullname, elapsed);
+#ifdef FORCE_DELETE_ONE_IMMEDIATELY
+	if (1 || elapsed > trashsec) {
+#else
 	if (elapsed > trashsec) {
+#endif
 		if (unlink(fullname)) {
 			bbs_error("unlink(%s) failed: %s\n", fullname, strerror(errno));
 		} else {
@@ -72,6 +79,9 @@ static int on_mailbox_trash(const char *dir_name, const char *filename, int seqn
 		}
 		maildir_parse_uid_from_filename(filename, &msguid);
 		uintlist_append2(&traversal->a, &traversal->sa, &traversal->lengths, &traversal->allocsizes, msguid, (unsigned int) seqno);
+#ifdef FORCE_DELETE_ONE_IMMEDIATELY
+		return 1;
+#endif
 	}
 	return 0;
 }
@@ -80,6 +90,7 @@ static void scan_mailboxes(void)
 {
 	DIR *dir;
 	struct dirent *entry;
+	char fulldir[512];
 	char trashdir[515];
 
 	/* Traverse each mailbox top-level maildir. The order of maildir traversal does not matter. */
@@ -109,7 +120,8 @@ static void scan_mailboxes(void)
 		traversal.mbox = mbox;
 		maildir_ordered_traverse(trashdir, on_mailbox_trash, &traversal); /* Traverse files in the Trash folder */
 		if (traversal.lengths) {
-			maildir_indicate_expunged(EVENT_MESSAGE_EXPIRE, NULL, mbox, entry->d_name, traversal.a, traversal.sa, traversal.lengths, 0);
+			snprintf(fulldir, sizeof(fulldir), "%s/%s/cur", mailbox_maildir(NULL), entry->d_name); /* Construct the full curdir path for this maildir */
+			maildir_indicate_expunged(EVENT_MESSAGE_EXPIRE, NULL, mbox, fulldir, traversal.a, traversal.sa, traversal.lengths, 0);
 			free_if(traversal.a);
 			free_if(traversal.sa);
 		}

nets/net_imap.c

@@ -376,24 +376,20 @@ static int generate_status(struct imap_session *imap, const char *folder, char *
 
 static int generate_mailbox_name(struct imap_session *imap, const char *restrict maildir, char *restrict buf, size_t len)
 {
-	size_t rootlen, fulllen;
+	size_t rootlen;
 	unsigned int userid;
 	unsigned int muserid;
 	const char *fulldir = maildir;
-	const char *rootdir = mailbox_maildir(NULL);
 	int acl = 0;
 
 	if (!bbs_user_is_registered(imap->node->user)) {
 		return -1;
 	}
 
 	userid = imap->node->user->id;
-	rootlen = strlen(rootdir);
-	fulllen = strlen(fulldir);
+	rootlen = strlen(mailbox_maildir(NULL));
 
-	if (fulllen <= rootlen) {
-		bbs_error("Maildir length %lu <= root maildir length %lu?\n", fulllen, rootlen);
-		bbs_soft_assert(0);
+	if (mailbox_maildir_validate(fulldir)) {
 		return -1;
 	}
 
@@ -403,7 +399,8 @@ static int generate_mailbox_name(struct imap_session *imap, const char *restrict
 	}
 
 	if (strlen_zero(fulldir)) {
-		bbs_error("Invalid maildir\n");
+		bbs_error("Empty maildir?\n");
+		bbs_soft_assert(0);
 		return -1;
 	}