From: Andre Noll <maan@tuebingen.mpg.de>
Date: Mon, 26 Dec 2016 22:50:34 +0000 (+0100)
Subject: load_chunk_table(): Don't trust afhi->chunks_total.
X-Git-Tag: v0.6.0~7^2~22
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=5afe721aeb241bd21b832994ec2f2b7d8ff84766

load_chunk_table(): Don't trust afhi->chunks_total.

Both callers pass a buffer to load_chunk_table() to initialize
afhi->chunk_table. The function does not check the size of the passed
buffer but relies on afhi->chunks_total to tell how many values to
copy from the buffer.

In other words, the buffer size gives an upper bound for the number of
chunks in the chunk table, but this bould is not checked. This patch
makes the code more robust by adding a suitable check. To this aim,
load_chunk_table() is changed to receive a pointer to an osl object
(which contains the buffer size) rather than only the buffer.

One caller, load_afd(), initializes the chunk table from a shared
memory area buffer. It does not know or care about the buffer size
so far. We introduce the the new IPC helper shm_size() to let it tell
the size of the area and pass it to load_chunk_table().
---

diff --git a/aft.c b/aft.c
index 8b9d85b5..1756ec04 100644
--- a/aft.c
+++ b/aft.c
@@ -437,13 +437,13 @@ static uint32_t save_chunk_table(struct afh_info *afhi, char *buf)
 	return max;
 }
 
-static void load_chunk_table(struct afh_info *afhi, char *buf)
+static void load_chunk_table(struct afh_info *afhi, const struct osl_object *ct)
 {
 	int i;
 
 	afhi->chunk_table = para_malloc(sizeof_chunk_table(afhi));
-	for (i = 0; i <= afhi->chunks_total; i++)
-		afhi->chunk_table[i] = read_u32(buf + 4 * i);
+	for (i = 0; i <= afhi->chunks_total && i * 4 + 3 < ct->size; i++)
+		afhi->chunk_table[i] = read_u32(ct->data + 4 * i);
 }
 
 /**
@@ -663,14 +663,22 @@ int load_afd(int shmid, struct audio_file_data *afd)
 {
 	void *shm_afd;
 	int ret;
+	struct osl_object obj;
 
 	ret = shm_attach(shmid, ATTACH_RO, &shm_afd);
 	if (ret < 0)
 		return ret;
+	ret = shm_size(shmid, &obj.size);
+	if (ret < 0)
+		goto detach;
 	*afd = *(struct audio_file_data *)shm_afd;
-	load_chunk_table(&afd->afhi, shm_afd + sizeof(*afd));
+	obj.data = shm_afd + sizeof(*afd);
+	obj.size -= sizeof(*afd);
+	load_chunk_table(&afd->afhi, &obj);
+	ret = 1;
+detach:
 	shm_detach(shm_afd);
-	return 1;
+	return ret;
 }
 
 static int get_local_time(uint64_t *seconds, char *buf, size_t size,
@@ -1076,7 +1084,7 @@ again:
 	save_afsi(&new_afsi, &afsi_obj); /* in-place update */
 
 	afd->audio_format_id = d->afsi.audio_format_id;
-	load_chunk_table(&afd->afhi, chunk_table_obj.data);
+	load_chunk_table(&afd->afhi, &chunk_table_obj);
 	aced.aft_row = current_aft_row;
 	aced.old_afsi = &d->afsi;
 	/*
diff --git a/ipc.c b/ipc.c
index 9488224a..d7f515df 100644
--- a/ipc.c
+++ b/ipc.c
@@ -161,6 +161,27 @@ int shm_attach(int id, enum shm_attach_mode mode, void **result)
 	return *result == (void *) -1? -ERRNO_TO_PARA_ERROR(errno) : 1;
 }
 
+/**
+ * Get the size of a shared memory segment.
+ *
+ * \param id The shared memory segment identifier.
+ * \param result Size in bytes is returned here, zero on errors.
+ *
+ * \return Standard.
+ *
+ * \sa shmctl(2).
+ */
+int shm_size(int id, size_t *result)
+{
+	struct shmid_ds ds; /* data structure */
+
+	*result = 0;
+	if (shmctl(id, IPC_STAT, &ds) < 0)
+		return -ERRNO_TO_PARA_ERROR(errno);
+	*result = ds.shm_segsz;
+	return 1;
+}
+
 /**
  * Detach a shared memory segment.
  *
diff --git a/ipc.h b/ipc.h
index c8d31c0c..c4437104 100644
--- a/ipc.h
+++ b/ipc.h
@@ -11,4 +11,5 @@ int shm_new(size_t size);
 int shm_attach(int id, enum shm_attach_mode mode, void **result);
 int shm_detach(void *addr);
 int shm_destroy(int id);
+int shm_size(int id, size_t *result);
 size_t shm_get_shmmax(void);