From a38b1cbc5e917d945340a6dd9dba4274a2eb8789 Mon Sep 17 00:00:00 2001 From: David Oberhollenzer Date: Fri, 23 Aug 2019 13:23:58 +0200 Subject: Size accounting + alloc() overflow checking, round #2 Signed-off-by: David Oberhollenzer --- lib/fstree/fstree_from_dir.c | 9 ++++++++- lib/fstree/get_path.c | 16 ++++++++++++--- lib/fstree/mknode.c | 40 +++++++++++++++++++++++++++++-------- lib/sqfs/read_inode.c | 15 ++++++++++---- lib/sqfs/read_table.c | 7 +++---- lib/sqfs/write_export_table.c | 7 +++++-- lib/sqfs/write_table.c | 4 ++-- lib/sqfs/write_xattr.c | 2 +- lib/sqfs/xattr_reader.c | 46 ++++++++++++++++++++++++++++--------------- 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 #include +#include 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 #include #include +#include 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; -- cgit v1.2.3