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/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 ++++++++++++++++++++++++++++--------------- 6 files changed, 52 insertions(+), 29 deletions(-) (limited to 'lib/sqfs') 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