Size accounting + alloc() overflow checking, round #2

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

6 files changed, 52 insertions, 29 deletions

diff --git a/lib/sqfs/read_inode.c b/lib/sqfs/read_inode.c
index 79c5a55..e4b4dfa 100644
--- a/lib/sqfs/read_inode.c
+++ b/lib/sqfs/read_inode.c
@@ -159,6 +159,7 @@ static sqfs_inode_generic_t *read_inode_slink(meta_reader_t *ir,
 {
 	sqfs_inode_generic_t *out;
 	sqfs_inode_slink_t slink;
+	size_t size;
 
 	if (meta_reader_read(ir, &slink, sizeof(slink)))
 		return NULL;
@@ -166,12 +167,15 @@ static sqfs_inode_generic_t *read_inode_slink(meta_reader_t *ir,
 	SWAB32(slink.nlink);
 	SWAB32(slink.target_size);
 
-	out = calloc(1, sizeof(*out) + slink.target_size + 1);
-	if (out == NULL) {
-		perror("reading symlink inode");
-		return NULL;
+	if (SZ_ADD_OV(slink.target_size, 1, &size) ||
+	    SZ_ADD_OV(sizeof(*out), size, &size)) {
+		goto fail;
 	}
 
+	out = calloc(1, size);
+	if (out == NULL)
+		goto fail;
+
 	out->slink_target = (char *)out->extra;
 	out->base = *base;
 	out->data.slink = slink;
@@ -182,6 +186,9 @@ static sqfs_inode_generic_t *read_inode_slink(meta_reader_t *ir,
 	}
 
 	return out;
+fail:
+	perror("reading symlink inode");
+	return NULL;
 }
 
 static sqfs_inode_generic_t *read_inode_slink_ext(meta_reader_t *ir,
diff --git a/lib/sqfs/read_table.c b/lib/sqfs/read_table.c
index 6efd401..d486c97 100644
--- a/lib/sqfs/read_table.c
+++ b/lib/sqfs/read_table.c
@@ -18,7 +18,7 @@ void *sqfs_read_table(int fd, compressor_t *cmp, size_t table_size,
 		      uint64_t location, uint64_t lower_limit,
 		      uint64_t upper_limit)
 {
-	size_t diff, block_count, list_size, blk_idx = 0;
+	size_t diff, block_count, blk_idx = 0;
 	uint64_t start, *locations;
 	meta_reader_t *m;
 	void *data, *ptr;
@@ -35,8 +35,7 @@ void *sqfs_read_table(int fd, compressor_t *cmp, size_t table_size,
 	if ((table_size % SQFS_META_BLOCK_SIZE) != 0)
 		++block_count;
 
-	list_size = sizeof(uint64_t) * block_count;
-	locations = malloc(list_size);
+	locations = alloc_array(sizeof(uint64_t), block_count);
 
 	if (locations == NULL) {
 		perror("allocation table location list");
@@ -44,7 +43,7 @@ void *sqfs_read_table(int fd, compressor_t *cmp, size_t table_size,
 	}
 
 	if (read_data_at("reading table locations", location,
-			 fd, locations, list_size)) {
+			 fd, locations, sizeof(uint64_t) * block_count)) {
 		goto fail_idx;
 	}
 
diff --git a/lib/sqfs/write_export_table.c b/lib/sqfs/write_export_table.c
index f81632a..e42df15 100644
--- a/lib/sqfs/write_export_table.c
+++ b/lib/sqfs/write_export_table.c
@@ -19,8 +19,10 @@ int write_export_table(int outfd, fstree_t *fs, sqfs_super_t *super,
 	size_t i, size;
 	int ret;
 
-	size = sizeof(uint64_t) * (fs->inode_tbl_size - 1);
-	table = malloc(size);
+	if (fs->inode_tbl_size < 1)
+		return 0;
+
+	table = alloc_array(sizeof(uint64_t), (fs->inode_tbl_size - 1));
 
 	if (table == NULL) {
 		perror("Allocating NFS export table");
@@ -35,6 +37,7 @@ int write_export_table(int outfd, fstree_t *fs, sqfs_super_t *super,
 		}
 	}
 
+	size = sizeof(uint64_t) * (fs->inode_tbl_size - 1);
 	ret = sqfs_write_table(outfd, super, cmp, table, size, &start);
 
 	super->export_table_start = start;
diff --git a/lib/sqfs/write_table.c b/lib/sqfs/write_table.c
index 2e55a26..aea5058 100644
--- a/lib/sqfs/write_table.c
+++ b/lib/sqfs/write_table.c
@@ -27,8 +27,7 @@ int sqfs_write_table(int outfd, sqfs_super_t *super, compressor_t *cmp,
 	if ((table_size % SQFS_META_BLOCK_SIZE) != 0)
 		++block_count;
 
-	list_size = sizeof(uint64_t) * block_count;
-	locations = malloc(list_size);
+	locations = alloc_array(sizeof(uint64_t), block_count);
 
 	if (locations == NULL) {
 		perror("writing table");
@@ -64,6 +63,7 @@ int sqfs_write_table(int outfd, sqfs_super_t *super, compressor_t *cmp,
 	/* write location list */
 	*start = super->bytes_used;
 
+	list_size = sizeof(uint64_t) * block_count;
 	if (write_data("writing table locations", outfd, locations, list_size))
 		goto out;
 
diff --git a/lib/sqfs/write_xattr.c b/lib/sqfs/write_xattr.c
index 7214ea7..2263fbe 100644
--- a/lib/sqfs/write_xattr.c
+++ b/lib/sqfs/write_xattr.c
@@ -215,7 +215,7 @@ int write_xattr(int outfd, fstree_t *fs, sqfs_super_t *super,
 	if ((count * sizeof(id_ent)) % SQFS_META_BLOCK_SIZE)
 		++blocks;
 
-	tbl = calloc(sizeof(uint64_t), blocks);
+	tbl = alloc_array(sizeof(uint64_t), blocks);
 
 	if (tbl == NULL) {
 		perror("generating xattr ID table");
diff --git a/lib/sqfs/xattr_reader.c b/lib/sqfs/xattr_reader.c
index 7a881c7..399940f 100644
--- a/lib/sqfs/xattr_reader.c
+++ b/lib/sqfs/xattr_reader.c
@@ -14,6 +14,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
+#include <errno.h>
 
 struct xattr_reader_t {
 	uint64_t xattr_start;
@@ -54,7 +55,7 @@ static int get_id_block_locations(xattr_reader_t *xr, int sqfsfd,
 	if ((xr->num_ids * sizeof(sqfs_xattr_id_t)) % SQFS_META_BLOCK_SIZE)
 		xr->num_id_blocks += 1;
 
-	xr->id_block_starts = calloc(sizeof(uint64_t), xr->num_id_blocks);
+	xr->id_block_starts = alloc_array(sizeof(uint64_t), xr->num_id_blocks);
 	if (xr->id_block_starts == NULL) {
 		perror("allocating xattr ID location table");
 		return -1;
@@ -63,7 +64,7 @@ static int get_id_block_locations(xattr_reader_t *xr, int sqfsfd,
 	if (read_data_at("reading xattr ID block locations",
 			 super->xattr_id_table_start + sizeof(idtbl),
 			 sqfsfd, xr->id_block_starts,
-			 sizeof(xr->id_block_starts[0]) * xr->num_id_blocks)) {
+			 sizeof(uint64_t) * xr->num_id_blocks)) {
 		goto fail;
 	}
 
@@ -127,7 +128,7 @@ static sqfs_xattr_entry_t *read_key(xattr_reader_t *xr)
 {
 	sqfs_xattr_entry_t key, *out;
 	const char *prefix;
-	size_t plen;
+	size_t plen, total;
 
 	if (meta_reader_read(xr->kvrd, &key, sizeof(key)))
 		return NULL;
@@ -143,10 +144,16 @@ static sqfs_xattr_entry_t *read_key(xattr_reader_t *xr)
 	}
 
 	plen = strlen(prefix);
-	out = calloc(1, sizeof(*out) + plen + key.size + 1);
+
+	if (SZ_ADD_OV(plen, key.size, &total) || SZ_ADD_OV(total, 1, &total) ||
+	    SZ_ADD_OV(sizeof(*out), total, &total)) {
+		errno = EOVERFLOW;
+		goto fail_alloc;
+	}
+
+	out = calloc(1, total);
 	if (out == NULL) {
-		perror("restoring xattr key");
-		return NULL;
+		goto fail_alloc;
 	}
 
 	*out = key;
@@ -158,14 +165,17 @@ static sqfs_xattr_entry_t *read_key(xattr_reader_t *xr)
 	}
 
 	return out;
+fail_alloc:
+	perror("allocating xattr key");
+	return NULL;
 }
 
 static sqfs_xattr_value_t *read_value(xattr_reader_t *xr,
 				      const sqfs_xattr_entry_t *key)
 {
+	size_t offset, new_offset, size;
 	sqfs_xattr_value_t value, *out;
 	uint64_t ref, start, new_start;
-	size_t offset, new_offset;
 
 	if (meta_reader_read(xr->kvrd, &value, sizeof(value)))
 		return NULL;
@@ -197,12 +207,16 @@ static sqfs_xattr_value_t *read_value(xattr_reader_t *xr,
 
 	value.size = le32toh(value.size);
 
-	out = calloc(1, sizeof(*out) + value.size);
-	if (out == NULL) {
-		perror("reading xattr value");
-		return NULL;
+	if (SZ_ADD_OV(sizeof(*out), value.size, &size) ||
+	    SZ_ADD_OV(size, 1, &size)) {
+		errno = EOVERFLOW;
+		goto fail_alloc;
 	}
 
+	out = calloc(1, size);
+	if (out == NULL)
+		goto fail_alloc;
+
 	*out = value;
 
 	if (meta_reader_read(xr->kvrd, out->value, value.size))
@@ -214,6 +228,9 @@ static sqfs_xattr_value_t *read_value(xattr_reader_t *xr,
 	}
 
 	return out;
+fail_alloc:
+	perror("allocating xattr value");
+	return NULL;
 fail:
 	free(out);
 	return NULL;
@@ -283,7 +300,6 @@ int xattr_reader_restore_node(xattr_reader_t *xr, fstree_t *fs,
 {
 	sqfs_xattr_id_t desc;
 	tree_xattr_t *it;
-	size_t size;
 
 	if (xr->kvrd == NULL || xr->idrd == NULL)
 		return 0;
@@ -301,10 +317,8 @@ int xattr_reader_restore_node(xattr_reader_t *xr, fstree_t *fs,
 	if (get_xattr_desc(xr, xattr, &desc))
 		return -1;
 
-	size = sizeof(*node->xattr);
-	size += sizeof(node->xattr->attr[0]) * desc.count;
-
-	node->xattr = calloc(1, size);
+	node->xattr = alloc_flex(sizeof(*node->xattr),
+				 sizeof(node->xattr->attr[0]), desc.count);
 	if (node->xattr == NULL) {
 		perror("creating xattr structure");
 		return -1;