* Fix memory corruption and lmtp handling

* Add e-mail address validation in lmtp

CMakeLists.txt

@@ -147,9 +147,16 @@ CHECK_INCLUDE_FILES(netdb.h  HAVE_NETDB_H)
 CHECK_INCLUDE_FILES(syslog.h HAVE_SYSLOG_H)
 CHECK_INCLUDE_FILES(libgen.h HAVE_LIBGEN_H)
 
+IF(HAVE_SYS_WAIT_H)
+	LIST(APPEND CMAKE_REQUIRED_INCLUDES sys/wait.h)
+ENDIF(HAVE_SYS_WAIT_H)
+
 CHECK_FUNCTION_EXISTS(setproctitle HAVE_SETPROCTITLE)
 CHECK_FUNCTION_EXISTS(getpagesize HAVE_GETPAGESIZE)
 CHECK_FUNCTION_EXISTS(nanosleep HAVE_NANOSLEEP)
+CHECK_FUNCTION_EXISTS(vfork HAVE_VFORK)
+CHECK_FUNCTION_EXISTS(wait4 HAVE_WAIT4)
+CHECK_FUNCTION_EXISTS(waitpid HAVE_WAITPID)
 
 CHECK_SYMBOL_EXISTS(PATH_MAX limits.h HAVE_PATH_MAX)
 CHECK_SYMBOL_EXISTS(MAXPATHLEN sys/param.h HAVE_MAXPATHLEN)

config.h.in

@@ -72,6 +72,12 @@
 
 #cmakedefine HAVE_SC_NPROCESSORS_ONLN 1
 
+#cmakedefine HAVE_VFORK          1
+
+#cmakedefine HAVE_WAIT4          1
+
+#cmakedefine HAVE_WAITPID        1
+
 #cmakedefine DEBUG_MODE          1
 
 #define RVERSION          "${RSPAMD_VERSION}"

src/controller.c

@@ -194,14 +194,14 @@ process_command (struct controller_command *cmd, char **cmd_args, struct control
 							  session->worker->srv->stat->connections_count);
 				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Control connections count: %u" CRLF,
 							  session->worker->srv->stat->control_connections_count);
-				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Bytes allocated: %zd" CRLF,
-							  mem_st.bytes_allocated);
-				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Memory chunks allocated: %zd" CRLF,
-							  mem_st.chunks_allocated);
-				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Shared chunks allocated: %zd" CRLF,
-							  mem_st.shared_chunks_allocated);
-				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Chunks freed: %zd" CRLF,
-							  mem_st.chunks_freed);
+				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Bytes allocated: %ld" CRLF,
+							  (long int)mem_st.bytes_allocated);
+				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Memory chunks allocated: %ld" CRLF,
+							  (long int)mem_st.chunks_allocated);
+				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Shared chunks allocated: %ld" CRLF,
+							  (long int)mem_st.shared_chunks_allocated);
+				r += snprintf (out_buf + r, sizeof (out_buf) - r, "Chunks freed: %ld" CRLF,
+							  (long int)mem_st.chunks_freed);
 				rspamd_dispatcher_write (session->dispatcher, out_buf, r, FALSE);
 			}
 			break;

src/fstring.c

@@ -190,14 +190,16 @@ fstrcpy (f_str_t *dest, f_str_t *src)
 size_t
 fstrcat (f_str_t *dest, f_str_t *src)
 {
-	register size_t cur = src->len;
+	register size_t cur = 0;
+	char *p = dest->begin + dest->len;
 
 	if (dest->size < src->len + dest->len) {
 		return 0;
 	}
 
-	while (cur < src->len && cur < dest->size) {
-		*(dest->begin + cur) = *(src->begin + cur);
+	while (cur < src->len) {
+		*p = *(src->begin + cur);
+		p ++;
 		cur ++;
 	}
 
@@ -246,14 +248,7 @@ fstralloc (memory_pool_t *pool, size_t len)
 {
 	f_str_t *res = memory_pool_alloc (pool, sizeof (f_str_t));
 
-	if (res == NULL) {
-		return NULL;
-	}
 	res->begin = memory_pool_alloc (pool, len);
-	if (res->begin == NULL) {
-		free (res);
-		return NULL;
-	}
 
 	res->size = len;
     res->len = 0;

src/lmtp.c

@@ -260,6 +260,12 @@ start_lmtp_worker (struct rspamd_worker *worker)
 
 	init_signals (&signals, sig_handler);
 	sigprocmask (SIG_UNBLOCK, &signals.sa_mask, NULL);
+	/* Ignore SIGPIPE for further use in LDA delivery */
+	sigemptyset (&signals.sa_mask);
+	sigaddset (&signals.sa_mask, SIGPIPE);
+	signals.sa_handler = SIG_IGN;
+	sigaction (SIGPIPE, &signals, NULL);
+	sigprocmask (SIG_UNBLOCK, &signals.sa_mask, NULL);
 
 	/* SIGUSR2 handler */
 	signal_set (&worker->sig_ev, SIGUSR2, sigusr_handler, (void *) worker);

src/lmtp_proto.c

@@ -53,6 +53,36 @@ static f_str_t data_dot = {
 	.len = sizeof (".\r\n") - 1
 };
 
+static const char *mail_regexp = "[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?";
+static GRegex *mail_re = NULL;
+
+/*
+ * Extract e-mail from read line 
+ * return <> if no valid address detected
+ */
+static char *
+extract_mail (memory_pool_t *pool, f_str_t *line)
+{
+	GError *err = NULL;
+	char *match;
+	GMatchInfo *info;
+
+	if (mail_re == NULL) {
+		/* Compile regexp */
+		mail_re = g_regex_new (mail_regexp, G_REGEX_RAW, 0, &err);
+	}
+
+	if (g_regex_match_full (mail_re, line->begin, line->len, 0, 0, &info, NULL) == TRUE) {
+		match = memory_pool_strdup (pool, g_match_info_fetch (info, 0));
+		g_match_info_free (info);
+	}
+	else {
+		match = "<>";
+	}
+
+	return match;
+}
+
 static void
 out_lmtp_reply (struct rspamd_lmtp_proto *lmtp, int code, char *rcode, char *msg)
 {
@@ -72,6 +102,7 @@ int
 read_lmtp_input_line (struct rspamd_lmtp_proto *lmtp, f_str_t *line)
 {
 	char *c, *rcpt;
+	f_str_t fstr;
 	unsigned int i = 0, l = 0, size;
 
 	switch (lmtp->state) {
@@ -108,19 +139,9 @@ read_lmtp_input_line (struct rspamd_lmtp_proto *lmtp, f_str_t *line)
 			else {
 				i += mail_command.len;
 				c = line->begin + i;
-				/* Get data from brackets (<>)*/
-				while (*c++ != '<' && i < line->len) {
-					i ++;
-				}
-				while (*c != '>' && i < line->len) {
-					l ++;
-					c ++;
-					i ++;
-				}
-
-				lmtp->task->from = memory_pool_alloc (lmtp->task->task_pool, l + 1);
-				/* Strlcpy makes string null terminated by design */
-				g_strlcpy (lmtp->task->from, c - l, l + 1);
+				fstr.begin = line->begin + i;
+				fstr.len = line->len - i;
+				lmtp->task->from = extract_mail (lmtp->task->task_pool, &fstr);
 				lmtp->state = LMTP_READ_RCPT;
 				out_lmtp_reply (lmtp, LMTP_OK, "2.1.0", "Sender ok");
 				return 0;
@@ -130,24 +151,22 @@ read_lmtp_input_line (struct rspamd_lmtp_proto *lmtp, f_str_t *line)
 			/* Search RCPT_TO: line */
 			if ((i = fstrstri (line, &rcpt_command)) == -1) {
 				msg_info ("read_lmtp_input_line: RCPT expected but not found");
-				out_lmtp_reply (lmtp, LMTP_BAD_CMD, "5.0.0", "Need RCPT here");
+				out_lmtp_reply (lmtp, LMTP_NO_RCPT, "5.5.4", "Need RCPT here");
 				return -1;
 			}
 			else {
 				i += rcpt_command.len;
 				c = line->begin + i;
-				/* Get data from brackets (<>)*/
-				while (*c++ != '<' && i < line->len) {
-					i ++;
+				fstr.begin = line->begin + i;
+				fstr.len = line->len - i;
+				rcpt = extract_mail (lmtp->task->task_pool, &fstr);
+				if (*rcpt == '<' && *(rcpt + 1) == '>') {
+					/* Invalid or empty rcpt not allowed */
+					msg_info ("read_lmtp_input_line: bad recipient");
+					out_lmtp_reply (lmtp, LMTP_NO_RCPT, "5.5.4", "Bad recipient");
+					return -1;
 				}
-				while (*c != '>' && i < line->len) {
-					l ++;
-					c ++;
-					i ++;
-				}
-				rcpt = memory_pool_alloc (lmtp->task->task_pool, l + 1);
 				/* Strlcpy makes string null terminated by design */
-				g_strlcpy (rcpt, c - l, l + 1);
 				lmtp->task->rcpt = g_list_prepend (lmtp->task->rcpt, rcpt);
 				lmtp->state = LMTP_READ_DATA;
 				out_lmtp_reply (lmtp, LMTP_OK, "2.1.0", "Recipient ok");
@@ -162,7 +181,7 @@ read_lmtp_input_line (struct rspamd_lmtp_proto *lmtp, f_str_t *line)
 				return -1;
 			}
 			else {
-				i += rcpt_command.len;
+				i += data_command.len;
 				c = line->begin + i;
 				/* Skip spaces */
 				while (isspace (*c++)) {
@@ -299,43 +318,98 @@ format_lda_args (struct worker_task *task)
 static int
 lmtp_deliver_lda (struct worker_task *task)
 {
-	char *args;
-	FILE *lda;
+	char *args, **argv;
 	GMimeStream *stream;
-	int rc, ecode;
+	int rc, ecode, p[2], argc;
+	pid_t cpid, pid;
 
 	if ((args = format_lda_args (task)) == NULL) {
 		return -1;
 	}
+	
+	/* Format arguments in shell style */
+	if (!g_shell_parse_argv (args, &argc, &argv, NULL)) {
+		msg_info ("lmtp_deliver_lda: cannot parse arguments");
+		return -1;
+	}
 
-	lda = popen (args, "w");
-	if (lda == NULL) {
-		msg_info ("lmtp_deliver_lda: cannot deliver to lda, %m");
+	if (pipe (p) == -1) {
+		g_strfreev (argv);
+		msg_info ("lmtp_deliver_lda: cannot open pipe: %m");
 		return -1;
 	}
+	
+	/* Fork to exec LDA */
+#ifdef HAVE_VFORK
+	if ((cpid = vfork ()) == -1) {
+		g_strfreev (argv);
+		msg_info ("lmtp_deliver_lda: cannot fork: %m");
+		return -1;
+	}
+#else 
+	if ((cpid = fork ()) == -1) {
+		g_strfreev (argv);
+		msg_info ("lmtp_deliver_lda: cannot fork: %m");
+		return -1;
+	}
+#endif
 
-	stream = g_mime_stream_file_new (lda);
+	if (cpid == 0) {
+		/* Child process, close write pipe and keep only read one */
+		close (p[1]);
+		/* Set standart IO descriptors */
+		if (p[0] != STDIN_FILENO) {
+			(void)dup2(p[0], STDIN_FILENO);
+			(void)close(p[0]);
+		}
+
+		execv (argv[0], argv);
+		_exit (127);
+	}
+	
+	close (p[0]);
+	stream = g_mime_stream_fs_new (p[1]);
 
 	if (g_mime_object_write_to_stream ((GMimeObject *)task->message, stream) == -1) {
+		g_strfreev (argv);
 		msg_info ("lmtp_deliver_lda: cannot write stream to lda");
 		return -1;
 	}
 
-	rc = pclose (lda);
+	g_object_unref (stream);
+	close (p[1]);
+
+#if defined(HAVE_WAIT4)
+	do {
+		pid = wait4(cpid, &rc, 0, NULL);
+	} while (pid == -1 && errno == EINTR);
+#elif defined(HAVE_WAITPID)
+	do {
+		pid = waitpid(cpid, &rc, 0);
+	} while (pid == -1 && errno == EINTR);
+#else
+#error wait mechanisms are undefined
+#endif
 	if (rc == -1) {
+		g_strfreev (argv);
 		msg_info ("lmtp_deliver_lda: lda returned error code");
 		return -1;
 	}
 	else if (WIFEXITED (rc)) {
 		ecode = WEXITSTATUS (rc);
 		if (ecode == 0) {
+			g_strfreev (argv);
 			return 0;
 		}
 		else {
+			g_strfreev (argv);
 			msg_info ("lmtp_deliver_lda: lda returned error code %d", ecode);
 			return -1;
 		}
 	}
+
+	g_strfreev (argv);
+	return -1;
 }
 
 int

src/mem_pool.c

@@ -52,9 +52,12 @@ pthread_mutex_t stat_mtx = PTHREAD_MUTEX_INITIALIZER;
 static memory_pool_stat_t *mem_pool_stat = NULL;
 
 static struct _pool_chain *
-pool_chain_new (size_t size) 
+pool_chain_new (memory_pool_ssize_t size) 
 {
 	struct _pool_chain *chain;
+
+	g_assert (size > 0);
+
 	chain = g_malloc (sizeof (struct _pool_chain));
 	chain->begin = g_malloc (size);
 	chain->len = size;
@@ -68,7 +71,7 @@ pool_chain_new (size_t size)
 }
 
 static struct _pool_chain_shared *
-pool_chain_new_shared (size_t size) 
+pool_chain_new_shared (memory_pool_ssize_t size) 
 {
 	struct _pool_chain_shared *chain;
 
@@ -111,10 +114,11 @@ pool_chain_new_shared (size_t size)
  * @return new memory pool object
  */
 memory_pool_t* 
-memory_pool_new (size_t size)
+memory_pool_new (memory_pool_ssize_t size)
 {
 	memory_pool_t *new;
 	
+	g_assert (size > 0);
 	/* Allocate statistic structure if it is not allocated before */
 	if (mem_pool_stat == NULL) {
 #if defined(HAVE_MMAP_ANON)
@@ -142,7 +146,7 @@ memory_pool_new (size_t size)
 }
 
 void *
-memory_pool_alloc (memory_pool_t *pool, size_t size)
+memory_pool_alloc (memory_pool_t *pool, memory_pool_ssize_t size)
 {
 	u_char *tmp;
 	struct _pool_chain *new, *cur;
@@ -185,7 +189,7 @@ memory_pool_alloc (memory_pool_t *pool, size_t size)
 }
 
 void *
-memory_pool_alloc0 (memory_pool_t *pool, size_t size)
+memory_pool_alloc0 (memory_pool_t *pool, memory_pool_ssize_t size)
 {
 	void *pointer = memory_pool_alloc (pool, size);
 	if (pointer) {
@@ -197,7 +201,7 @@ memory_pool_alloc0 (memory_pool_t *pool, size_t size)
 char *
 memory_pool_strdup (memory_pool_t *pool, const char *src)
 {
-	size_t len;
+	memory_pool_ssize_t len;
 	char *newstr;
 
 	if (src == NULL) {
@@ -211,12 +215,13 @@ memory_pool_strdup (memory_pool_t *pool, const char *src)
 }
 
 void *
-memory_pool_alloc_shared (memory_pool_t *pool, size_t size)
+memory_pool_alloc_shared (memory_pool_t *pool, memory_pool_ssize_t size)
 {
 	u_char *tmp;
 	struct _pool_chain_shared *new, *cur;
 
 	if (pool) {
+		g_assert (size > 0);
 		cur = pool->shared_pool;
 		if (!cur) {
 			cur = pool_chain_new_shared (pool->first_pool->len);
@@ -384,7 +389,7 @@ memory_pool_stat (memory_pool_stat_t *st)
 }
 
 #define FIXED_POOL_SIZE 4095
-size_t
+memory_pool_ssize_t
 memory_pool_get_size ()
 {
 #ifdef HAVE_GETPAGESIZE

src/mem_pool.h

@@ -19,13 +19,18 @@
  */
 typedef void (*pool_destruct_func)(void *ptr);
 
+/**
+ * Type that represents allocating size
+ */
+typedef long int memory_pool_ssize_t;
+
 /**
  * Pool page structure
  */
 struct _pool_chain {
 	u_char *begin;					/**< begin of pool chain block 				*/
 	u_char *pos;					/**< current start of free space in block 	*/
-	size_t len;						/**< length of block 						*/
+	memory_pool_ssize_t len;						/**< length of block 						*/
 	struct _pool_chain *next;		/**< chain link 							*/
 };
 
@@ -35,7 +40,7 @@ struct _pool_chain {
 struct _pool_chain_shared {
 	u_char *begin;
 	u_char *pos;
-	size_t len;
+	memory_pool_ssize_t len;
 	gint lock;
 	struct _pool_chain_shared *next;
 };
@@ -63,10 +68,10 @@ typedef struct memory_pool_s {
  * Statistics structure
  */
 typedef struct memory_pool_stat_s {
-	size_t bytes_allocated;					/**< bytes that are allocated with pool allocator		*/
-	size_t chunks_allocated;				/**< number of chunks that are allocated				*/
-	size_t shared_chunks_allocated;			/**< shared chunks allocated							*/
-	size_t chunks_freed;					/**< chunks freed										*/
+	memory_pool_ssize_t bytes_allocated;					/**< bytes that are allocated with pool allocator		*/
+	memory_pool_ssize_t chunks_allocated;				/**< number of chunks that are allocated				*/
+	memory_pool_ssize_t shared_chunks_allocated;			/**< shared chunks allocated							*/
+	memory_pool_ssize_t chunks_freed;					/**< chunks freed										*/
 } memory_pool_stat_t;
 
 /**
@@ -82,7 +87,7 @@ typedef struct memory_pool_rwlock_s {
  * @param size size of pool's page
  * @return new memory pool object
  */
-memory_pool_t* memory_pool_new (size_t size);
+memory_pool_t* memory_pool_new (memory_pool_ssize_t size);
 
 /** 
  * Get memory from pool
@@ -90,7 +95,7 @@ memory_pool_t* memory_pool_new (size_t size);
  * @param size bytes to allocate
  * @return pointer to allocated object
  */
-void* memory_pool_alloc (memory_pool_t* pool, size_t size);
+void* memory_pool_alloc (memory_pool_t* pool, memory_pool_ssize_t size);
 
 /**
  * Get memory and set it to zero
@@ -98,7 +103,7 @@ void* memory_pool_alloc (memory_pool_t* pool, size_t size);
  * @param size bytes to allocate
  * @return pointer to allocated object
  */
-void* memory_pool_alloc0 (memory_pool_t* pool, size_t size);
+void* memory_pool_alloc0 (memory_pool_t* pool, memory_pool_ssize_t size);
 
 /**
  * Make a copy of string in pool
@@ -113,7 +118,7 @@ char* memory_pool_strdup (memory_pool_t* pool, const char *src);
  * @param pool memory pool object
  * @param size bytes to allocate
  */
-void* memory_pool_alloc_shared (memory_pool_t *pool, size_t size);
+void* memory_pool_alloc_shared (memory_pool_t *pool, memory_pool_ssize_t size);
 
 /**
  * Lock chunk of shared memory in which pointer is placed
@@ -203,7 +208,7 @@ void memory_pool_stat (memory_pool_stat_t *st);
  * Get optimal pool size based on page size for this system
  * @return size of memory page in system
  */
-size_t memory_pool_get_size ();
+memory_pool_ssize_t memory_pool_get_size ();
 
 /**
  * Macro that return free space in pool page