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

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

9 files changed, 105 insertions, 41 deletions

diff --git a/lib/fstree/fstree_from_dir.c b/lib/fstree/fstree_from_dir.c
index 9aab5df..d6fac86 100644
--- a/lib/fstree/fstree_from_dir.c
+++ b/lib/fstree/fstree_from_dir.c
@@ -25,6 +25,7 @@
 static char *get_file_path(tree_node_t *n, const char *name)
 {
 	char *ptr, *new;
+	size_t len;
 	int ret;
 
 	if (n->parent == NULL) {
@@ -41,7 +42,13 @@ static char *get_file_path(tree_node_t *n, const char *name)
 	ret = canonicalize_name(ptr);
 	assert(ret == 0);
 
-	new = realloc(ptr, strlen(ptr) + strlen(name) + 2);
+	if (SZ_ADD_OV(strlen(ptr), strlen(name), &len) ||
+	    SZ_ADD_OV(len, 2, &len)) {
+		errno = EOVERFLOW;
+		goto fail;
+	}
+
+	new = realloc(ptr, len);
 	if (new == NULL)
 		goto fail;
 
diff --git a/lib/fstree/get_path.c b/lib/fstree/get_path.c
index decf92e..f464ade 100644
--- a/lib/fstree/get_path.c
+++ b/lib/fstree/get_path.c
@@ -7,9 +7,11 @@
 #include "config.h"
 
 #include "fstree.h"
+#include "util.h"
 
 #include <string.h>
 #include <stdlib.h>
+#include <errno.h>
 
 char *fstree_get_path(tree_node_t *node)
 {
@@ -21,14 +23,19 @@ char *fstree_get_path(tree_node_t *node)
 		return strdup("/");
 
 	for (it = node; it != NULL && it->parent != NULL; it = it->parent) {
-		len += strlen(it->name) + 1;
+		if (SZ_ADD_OV(len, strlen(it->name), &len) ||
+		    SZ_ADD_OV(len, 1, &len))
+			goto fail_ov;
 	}
 
-	str = malloc(len + 1);
+	if (SZ_ADD_OV(len, 1, &len))
+		goto fail_ov;
+
+	str = malloc(len);
 	if (str == NULL)
 		return NULL;
 
-	ptr = str + len;
+	ptr = str + len - 1;
 	*ptr = '\0';
 
 	for (it = node; it != NULL && it->parent != NULL; it = it->parent) {
@@ -40,4 +47,7 @@ char *fstree_get_path(tree_node_t *node)
 	}
 
 	return str;
+fail_ov:
+	errno = EOVERFLOW;
+	return NULL;
 }
diff --git a/lib/fstree/mknode.c b/lib/fstree/mknode.c
index 9132458..1c3b3a0 100644
--- a/lib/fstree/mknode.c
+++ b/lib/fstree/mknode.c
@@ -17,7 +17,7 @@ tree_node_t *fstree_mknode(fstree_t *fs, tree_node_t *parent, const char *name,
 			   size_t name_len, const char *extra,
 			   const struct stat *sb)
 {
-	size_t size = sizeof(tree_node_t), block_count = 0;
+	size_t size = sizeof(tree_node_t), block_count = 0, total;
 	tree_node_t *n;
 	char *ptr;
 
@@ -27,24 +27,45 @@ tree_node_t *fstree_mknode(fstree_t *fs, tree_node_t *parent, const char *name,
 			errno = EINVAL;
 			return NULL;
 		}
-		size += strlen(extra) + 1;
+		if (SZ_ADD_OV(size, strlen(extra), &size) ||
+		    SZ_ADD_OV(size, 1, &size)) {
+			goto fail_ov;
+		}
 		break;
 	case S_IFDIR:
-		size += sizeof(*n->data.dir);
+		if (SZ_ADD_OV(size, sizeof(*n->data.dir), &size))
+			goto fail_ov;
 		break;
 	case S_IFREG:
 		block_count = (sb->st_size / fs->block_size);
 		if ((sb->st_size % fs->block_size) != 0)
 			++block_count;
 
-		size += sizeof(*n->data.file);
-		size += block_count * sizeof(n->data.file->blocks[0]);
-		if (extra != NULL)
-			size += strlen(extra) + 1;
+		if (SZ_MUL_OV(block_count, sizeof(n->data.file->blocks[0]),
+			      &total)) {
+			goto fail_ov;
+		}
+
+		if (SZ_ADD_OV(size, sizeof(*n->data.file), &size) ||
+		    SZ_ADD_OV(size, total, &size)) {
+			goto fail_ov;
+		}
+
+		if (extra != NULL) {
+			if (SZ_ADD_OV(size, strlen(extra), &size) ||
+			    SZ_ADD_OV(size, 1, &size)) {
+				goto fail_ov;
+			}
+		}
 		break;
 	}
 
-	n = calloc(1, size + name_len + 1);
+	if (SZ_ADD_OV(size, name_len, &total) ||
+	    SZ_ADD_OV(total, 1, &total)) {
+		goto fail_ov;
+	}
+
+	n = calloc(1, total);
 	if (n == NULL)
 		return NULL;
 
@@ -91,4 +112,7 @@ tree_node_t *fstree_mknode(fstree_t *fs, tree_node_t *parent, const char *name,
 	n->name = (char *)n + size;
 	memcpy(n->name, name, name_len);
 	return n;
+fail_ov:
+	errno = EOVERFLOW;
+	return NULL;
 }
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;