# HG changeset patch
# User Timo Sirainen <tss@iki.fi>
# Date 1053026541 -10800
# Node ID 8f56379c3917e0f7f18617428ea9880eb53eb5de
# Parent  f5e7019f6c469620aceb4b52bdb8bb8519677cda
Renamed buffer_*_space() to buffer_*_space_unsafe() and added several
warnings about using them. Fixed their usage in a few places in sources
where they could have produced invalid results (no buffer overflows,
luckily).

diff -r f5e7019f6c46 -r 8f56379c3917 src/auth/master-connection.c
--- a/src/auth/master-connection.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/auth/master-connection.c	Thu May 15 22:22:21 2003 +0300
@@ -37,36 +37,42 @@
 static struct auth_master_reply *
 fill_reply(const struct user_data *user, size_t *reply_size)
 {
-	struct auth_master_reply *reply;
+	struct auth_master_reply reply, *reply_p;
 	buffer_t *buf;
 	char *p;
 
 	buf = buffer_create_dynamic(data_stack_pool,
-				    sizeof(*reply) + 256, (size_t)-1);
-	reply = buffer_append_space(buf, sizeof(*reply));
+				    sizeof(reply) + 256, (size_t)-1);
+	memset(&reply, 0, sizeof(reply));
+	buffer_append(buf, &reply, sizeof(reply));
 
-	reply->success = TRUE;
+	reply.success = TRUE;
 
-	reply->uid = user->uid;
-	reply->gid = user->gid;
+	reply.uid = user->uid;
+	reply.gid = user->gid;
 
-	reply->system_user_idx = reply_add(buf, user->system_user);
-	reply->virtual_user_idx = reply_add(buf, user->virtual_user);
-	reply->mail_idx = reply_add(buf, user->mail);
+	reply.system_user_idx = reply_add(buf, user->system_user);
+	reply.virtual_user_idx = reply_add(buf, user->virtual_user);
+	reply.mail_idx = reply_add(buf, user->mail);
 
 	p = strstr(user->home, "/./");
 	if (p == NULL) {
-		reply->home_idx = reply_add(buf, user->home);
-		reply->chroot_idx = reply_add(buf, NULL);
+		reply.home_idx = reply_add(buf, user->home);
+		reply.chroot_idx = reply_add(buf, NULL);
 	} else {
 		/* wu-ftpd like <chroot>/./<home> */
-		reply->chroot_idx = reply_add(buf, t_strdup_until(user->home, p));
-		reply->home_idx = reply_add(buf, p + 3);
+		reply.chroot_idx =
+			reply_add(buf, t_strdup_until(user->home, p));
+		reply.home_idx = reply_add(buf, p + 3);
 	}
 
 	*reply_size = buffer_get_used_size(buf);
-	reply->data_size = *reply_size - sizeof(*reply);
-	return reply;
+	reply.data_size = *reply_size - sizeof(reply);
+
+	reply_p = buffer_get_space_unsafe(buf, 0, sizeof(reply));
+	*reply_p = reply;
+
+	return reply_p;
 }
 
 static void send_reply(struct auth_master_reply *reply, size_t reply_size,
diff -r f5e7019f6c46 -r 8f56379c3917 src/imap/commands.c
--- a/src/imap/commands.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/imap/commands.c	Thu May 15 22:22:21 2003 +0300
@@ -57,11 +57,11 @@
 
 void command_register(const char *name, command_func_t *func)
 {
-	struct command *cmd;
+	struct command cmd;
 
-	cmd = buffer_append_space(cmdbuf, sizeof(*cmd));
-	cmd->name = name;
-	cmd->func = func;
+	cmd.name = name;
+	cmd.func = func;
+	buffer_append(cmdbuf, &cmd, sizeof(cmd));
 
 	cmdbuf_unsorted = TRUE;
 }
diff -r f5e7019f6c46 -r 8f56379c3917 src/imap/imap-sort.c
--- a/src/imap/imap-sort.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/imap/imap-sort.c	Thu May 15 22:22:21 2003 +0300
@@ -374,7 +374,8 @@
 	if (ctx->common_mask != 0)
 		mail_sort_check_flush(ctx, mail);
 
-	buf = buffer_append_space(ctx->sort_buffer, ctx->sort_element_size);
+	buf = buffer_append_space_unsafe(ctx->sort_buffer,
+					 ctx->sort_element_size);
 	id = ctx->id_is_uid ? mail->uid : mail->seq;
 	memcpy(buf, &id, sizeof(id)); pos = sizeof(id);
 
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib-charset/charset-iconv.c
--- a/src/lib-charset/charset-iconv.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib-charset/charset-iconv.c	Thu May 15 22:22:21 2003 +0300
@@ -85,7 +85,7 @@
 	size = destleft;
 	srcleft = *src_size;
 	ic_srcbuf = (ICONV_CONST char *) src;
-	ic_destbuf = buffer_append_space(dest, destleft);
+	ic_destbuf = buffer_append_space_unsafe(dest, destleft);
 
 	if (iconv(t->cd, &ic_srcbuf, &srcleft,
 		  &ic_destbuf, &destleft) != (size_t)-1)
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib-charset/charset-utf8.c
--- a/src/lib-charset/charset-utf8.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib-charset/charset-utf8.c	Thu May 15 22:22:21 2003 +0300
@@ -12,7 +12,7 @@
 	char *destbuf;
 	size_t i;
 
-	destbuf = buffer_get_space(dest, destpos, src_size);
+	destbuf = buffer_get_space_unsafe(dest, destpos, src_size);
 	for (i = 0; i < src_size; i++)
 		destbuf[i] = i_toupper(src[i]); /* FIXME: utf8 */
 }
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib-index/mail-index-update.c
--- a/src/lib-index/mail-index-update.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib-index/mail-index-update.c	Thu May 15 22:22:21 2003 +0300
@@ -130,13 +130,11 @@
 static void *create_data_block(struct mail_index_update *update,
 			       size_t data_size, size_t extra_size)
 {
-        struct mail_index_data_record_header *dest_hdr;
-        struct mail_index_data_record *rec, *destrec;
+        struct mail_index_data_record *rec, destrec;
 	enum mail_data_field field;
 	buffer_t *buf;
 	const void *src;
-	size_t src_size;
-	size_t full_field_size;
+	size_t src_size, filler_size;
 	int i;
 
 	i_assert(data_size <= UINT_MAX);
@@ -144,16 +142,15 @@
 	buf = buffer_create_static_hard(update->pool, data_size);
 
 	/* set header */
-	dest_hdr = buffer_append_space(buf, sizeof(*dest_hdr));
-	memcpy(dest_hdr, &update->data_hdr, sizeof(*dest_hdr));
-	dest_hdr->data_size = data_size;
+	update->data_hdr.data_size = data_size;
+	buffer_append(buf, &update->data_hdr, sizeof(update->data_hdr));
 
 	/* set fields */
 	rec = mail_index_data_lookup(update->index->data, update->rec, 0);
 	for (i = 0, field = 1; field != DATA_FIELD_LAST; i++, field <<= 1) {
 		if (update->fields[i] != NULL) {
 			/* value was modified - use it */
-			full_field_size =
+			destrec.full_field_size =
 				get_max_align_size(update->field_sizes[i],
 						   update->field_extra_sizes[i],
 						   &extra_size);
@@ -161,20 +158,24 @@
 			src_size = update->field_sizes[i];
 		} else if (rec != NULL && rec->field == field) {
 			/* use the old value */
-			full_field_size = rec->full_field_size;
+			destrec.full_field_size = rec->full_field_size;
 			src = rec->data;
 			src_size = rec->full_field_size;
 		} else {
 			/* the field doesn't exist, jump to next */
 			continue;
 		}
-		i_assert((full_field_size % INDEX_ALIGN_SIZE) == 0);
+		i_assert((destrec.full_field_size % INDEX_ALIGN_SIZE) == 0);
+
+		destrec.field = field;
+		buffer_append(buf, &destrec, SIZEOF_MAIL_INDEX_DATA);
+		buffer_append(buf, src, src_size);
 
-		destrec = buffer_append_space(buf, SIZEOF_MAIL_INDEX_DATA +
-					      full_field_size);
-		destrec->field = field;
-		destrec->full_field_size = full_field_size;
-		memcpy(destrec->data, src, src_size);
+		filler_size = destrec.full_field_size - src_size;
+		if (filler_size != 0) {
+			buffer_set_used_size(buf, buffer_get_used_size(buf) +
+					     filler_size);
+		}
 
 		if (rec != NULL && rec->field == field) {
 			rec = mail_index_data_next(update->index->data,
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib-index/mail-modifylog.c
--- a/src/lib-index/mail-modifylog.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib-index/mail-modifylog.c	Thu May 15 22:22:21 2003 +0300
@@ -1061,7 +1061,7 @@
 				unsigned int *expunges_before)
 {
 	struct modify_log_record *rec;
-	struct modify_log_expunge *expunge;
+	struct modify_log_expunge expunge, *expunges;
 	buffer_t *buf;
 	size_t count;
 	unsigned int before, max_records;
@@ -1118,19 +1118,18 @@
 				return NULL;
 			}
 
-			expunge = buffer_append_space(buf, sizeof(*expunge));
-
 			if (rec->seq1 < first_seq) {
 				/* partial initial match, update
 				   before-counter */
 				before += first_seq - rec->seq1;
-				expunge->seq_count = rec->seq2 - first_seq + 1;
+				expunge.seq_count = rec->seq2 - first_seq + 1;
 			} else {
-				expunge->seq_count = rec->seq2 - rec->seq1 + 1;
+				expunge.seq_count = rec->seq2 - rec->seq1 + 1;
 			}
 
-			expunge->uid1 = rec->uid1;
-			expunge->uid2 = rec->uid2;
+			expunge.uid1 = rec->uid1;
+			expunge.uid2 = rec->uid2;
+			buffer_append(buf, &expunge, sizeof(expunge));
 		}
 
 		if (rec->seq1 <= last_seq) {
@@ -1146,19 +1145,17 @@
 	}
 
 	/* terminate the array */
-	expunge = buffer_append_space(buf, sizeof(*expunge));
-	memset(expunge, 0, sizeof(*expunge));
+	buffer_set_used_size(buf, buffer_get_used_size(buf) + sizeof(expunge));
 
 	/* extract the array from buffer */
-	count = buffer_get_used_size(buf)/sizeof(struct modify_log_expunge);
-	expunge = buffer_free_without_data(buf);
+	count = buffer_get_used_size(buf) / sizeof(expunge);
+	expunges = buffer_free_without_data(buf);
 
 	/* sort the UID array, not including the terminating 0 */
-	qsort(expunge, count-1, sizeof(struct modify_log_expunge),
-	      compare_expunge);
+	qsort(expunges, count-1, sizeof(expunge), compare_expunge);
 
 	*expunges_before = before;
-	return expunge;
+	return expunges;
 }
 
 const struct modify_log_expunge *
@@ -1170,7 +1167,7 @@
 	/* pretty much copy&pasted from sequence code above ..
 	   kind of annoying */
 	struct modify_log_record *rec;
-	struct modify_log_expunge *expunge;
+	struct modify_log_expunge expunge, *expunges;
 	buffer_t *buf;
 	size_t count;
 	unsigned int before, max_records;
@@ -1227,28 +1224,25 @@
 				return NULL;
 			}
 
-			expunge = buffer_append_space(buf, sizeof(*expunge));
-
-			expunge->uid1 = rec->uid1;
-			expunge->uid2 = rec->uid2;
-			expunge->seq_count = rec->seq2 -rec->seq1 + 1;
+			expunge.uid1 = rec->uid1;
+			expunge.uid2 = rec->uid2;
+			expunge.seq_count = rec->seq2 -rec->seq1 + 1;
+			buffer_append(buf, &expunge, sizeof(expunge));
 		}
 	}
 
 	/* terminate the array */
-	expunge = buffer_append_space(buf, sizeof(*expunge));
-	memset(expunge, 0, sizeof(*expunge));
+	buffer_set_used_size(buf, buffer_get_used_size(buf) + sizeof(expunge));
 
 	/* extract the array from buffer */
-	count = buffer_get_used_size(buf) / sizeof(struct modify_log_expunge);
-	expunge = buffer_free_without_data(buf);
+	count = buffer_get_used_size(buf) / sizeof(expunge);
+	expunges = buffer_free_without_data(buf);
 
 	/* sort the UID array, not including the terminating 0 */
-	qsort(expunge, count-1, sizeof(struct modify_log_expunge),
-	      compare_expunge);
+	qsort(expunges, count-1, sizeof(expunge), compare_expunge);
 
 	*expunges_before = before;
-	return expunge;
+	return expunges;
 }
 
 static unsigned int
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib-index/mbox/mbox-index.c
--- a/src/lib-index/mbox/mbox-index.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib-index/mbox/mbox-index.c	Thu May 15 22:22:21 2003 +0300
@@ -197,7 +197,7 @@
 static void mbox_parse_imapbase(const unsigned char *value, size_t len,
 				struct mbox_header_context *ctx)
 {
-	const char **flag, *str;
+	const char *flag, *str;
 	char *end;
 	buffer_t *buf;
 	size_t pos, start;
@@ -223,15 +223,15 @@
 
 	/* we're at the 3rd field now, which begins the list of custom flags */
 	buf = buffer_create_dynamic(data_stack_pool,
-				    MAIL_CUSTOM_FLAGS_COUNT, MAX_CUSTOM_FLAGS);
+				    MAIL_CUSTOM_FLAGS_COUNT *
+				    sizeof(const char *),
+				    MAX_CUSTOM_FLAGS * sizeof(const char *));
 	for (start = pos; ; pos++) {
 		if (pos == len || value[pos] == ' ' || value[pos] == '\t') {
 			if (start != pos) {
-				flag = buffer_append_space(buf, sizeof(*flag));
-				if (flag == NULL)
+				flag = t_strdup_until(value+start, value+pos);
+				if (buffer_append(buf, flag, sizeof(flag)) == 0)
 					break;
-
-				*flag = t_strdup_until(value+start, value+pos);
 			}
 			start = pos+1;
 
@@ -242,9 +242,8 @@
 
 	flags = MAIL_CUSTOM_FLAGS_MASK;
 	count = buffer_get_used_size(buf) / sizeof(const char *);
-	flag = buffer_free_without_data(buf);
 	ret = mail_custom_flags_fix_list(ctx->index->custom_flags, &flags,
-					 flag, count);
+					 buffer_free_without_data(buf), count);
 
 	t_pop();
 }
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib-mail/message-part-serialize.c
--- a/src/lib-mail/message-part-serialize.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib-mail/message-part-serialize.c	Thu May 15 22:22:21 2003 +0300
@@ -46,29 +46,39 @@
 static unsigned int
 _message_part_serialize(struct message_part *part, buffer_t *dest)
 {
-	struct serialized_message_part *spart;
-	unsigned int count = 0;
+	struct serialized_message_part spart, *spart_p;
+	unsigned int count, children_count;
+	size_t pos;
 
+	count = 0;
 	while (part != NULL) {
 		/* create serialized part */
-		spart = buffer_append_space(dest, sizeof(*spart));
-		memset(spart, 0, sizeof(*spart));
+		memset(&spart, 0, sizeof(spart));
+
+		spart.physical_pos = part->physical_pos;
 
-		spart->physical_pos = part->physical_pos;
+		spart.header_physical_size = part->header_size.physical_size;
+		spart.header_virtual_size = part->header_size.virtual_size;
+		spart.header_lines = part->header_size.lines;
 
-		spart->header_physical_size = part->header_size.physical_size;
-		spart->header_virtual_size = part->header_size.virtual_size;
-		spart->header_lines = part->header_size.lines;
+		spart.body_physical_size = part->body_size.physical_size;
+		spart.body_virtual_size = part->body_size.virtual_size;
+		spart.body_lines = part->body_size.lines;
 
-		spart->body_physical_size = part->body_size.physical_size;
-		spart->body_virtual_size = part->body_size.virtual_size;
-		spart->body_lines = part->body_size.lines;
+		spart.flags = part->flags;
 
-		spart->flags = part->flags;
-
+		buffer_append(dest, &spart, sizeof(spart));
 		if (part->children != NULL) {
-			spart->children_count =
+			pos = buffer_get_used_size(dest) - sizeof(spart);
+			children_count =
 				_message_part_serialize(part->children, dest);
+
+			/* note that we can't just save the pointer to
+			   our appended spart since it may change after
+			   child-appends have realloc()ed buffer memory. */
+			spart_p = buffer_get_space_unsafe(dest, pos,
+							  sizeof(spart));
+			spart_p->children_count = children_count;
 		}
 
 		count++;
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib/buffer.c
--- a/src/lib/buffer.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib/buffer.c	Thu May 15 22:22:21 2003 +0300
@@ -311,7 +311,7 @@
 			   src, src_pos, copy_size);
 }
 
-void *buffer_get_space(buffer_t *buf, size_t pos, size_t size)
+void *buffer_get_space_unsafe(buffer_t *buf, size_t pos, size_t size)
 {
 	if (!buffer_check_write(buf, &pos, &size, FALSE))
 		return NULL;
@@ -319,9 +319,9 @@
 	return buf->w_buffer + pos;
 }
 
-void *buffer_append_space(buffer_t *buf, size_t size)
+void *buffer_append_space_unsafe(buffer_t *buf, size_t size)
 {
-	return buffer_get_space(buf, buf->used - buf->start_pos, size);
+	return buffer_get_space_unsafe(buf, buf->used - buf->start_pos, size);
 }
 
 const void *buffer_get_data(const buffer_t *buf, size_t *used_size)
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib/buffer.h
--- a/src/lib/buffer.h	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib/buffer.h	Thu May 15 22:22:21 2003 +0300
@@ -1,6 +1,11 @@
 #ifndef __BUFFER_H
 #define __BUFFER_H
 
+/* WARNING: Be careful with functions that return pointers to data.
+   With dynamic buffers they are valid only as long as buffer is not
+   realloc()ed. You shouldn't rely on it being valid if you have modified
+   buffer in any way. */
+
 /* Create a static sized buffer. Writes past this size will simply not
    succeed. */
 buffer_t *buffer_create_static(pool_t pool, size_t size);
@@ -47,17 +52,19 @@
 			 size_t src_pos, size_t copy_size);
 
 /* Returns pointer to specified position in buffer, or NULL if there's not
-   enough space. */
-void *buffer_get_space(buffer_t *buf, size_t pos, size_t size);
+   enough space. WARNING: The returned address may become invalid if you add
+   more data to buffer. */
+void *buffer_get_space_unsafe(buffer_t *buf, size_t pos, size_t size);
 /* Increase the buffer usage by given size, and return a pointer to beginning
    of it, or NULL if there's not enough space in buffer. */
-void *buffer_append_space(buffer_t *buf, size_t size);
+void *buffer_append_space_unsafe(buffer_t *buf, size_t size);
 
 /* Returns pointer to beginning of buffer data. Current used size of buffer is
    stored in used_size if it's non-NULL. */
 const void *buffer_get_data(const buffer_t *buf, size_t *used_size);
 /* Like buffer_get_data(), but don't return it as const. Returns NULL if the
-   buffer is non-modifyable. */
+   buffer is non-modifyable. WARNING: The returned address may become invalid
+   if you add more data to buffer. */
 void *buffer_get_modifyable_data(const buffer_t *buf, size_t *used_size);
 
 /* Set the "used size" of buffer, ie. 0 would set the buffer empty.
diff -r f5e7019f6c46 -r 8f56379c3917 src/lib/str.c
--- a/src/lib/str.c	Wed May 14 21:37:07 2003 +0300
+++ b/src/lib/str.c	Thu May 15 22:22:21 2003 +0300
@@ -143,7 +143,7 @@
 	fmt = printf_string_fix_format(fmt);
 	append_len = printf_string_upper_bound(fmt, args);
 
-	buf = buffer_append_space(str, append_len);
+	buf = buffer_append_space_unsafe(str, append_len);
 
 #ifdef HAVE_VSNPRINTF
 	ret = vsnprintf(buf, append_len, fmt, args2);