From 94cfa86b56792e8b95e3807c8174e348d70eac59 Mon Sep 17 00:00:00 2001 From: David Oberhollenzer Date: Sat, 20 Jun 2020 18:52:46 +0200 Subject: Fix block bounds checking in libsquashfs data reader Instead of doing the fragile size comparison in both loops, simply bail from the function if offset is out of bounds, clamp the size to the available range of the file and abail if it is zero. As a result, a lot of checks can be removed and the function will not return data beyond EOF. This problem occoured with files that have a short last block instead of a fragment. Signed-off-by: David Oberhollenzer --- lib/sqfs/data_reader.c | 43 ++++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/lib/sqfs/data_reader.c b/lib/sqfs/data_reader.c index 7148a53..2a4bf5a 100644 --- a/lib/sqfs/data_reader.c +++ b/lib/sqfs/data_reader.c @@ -308,22 +308,23 @@ sqfs_s32 sqfs_data_reader_read(sqfs_data_reader_t *data, sqfs_inode_get_file_block_start(inode, &off); block_count = sqfs_inode_get_file_block_count(inode); - /* find location of the first block */ - i = 0; + if (offset >= filesz) + return 0; - while (offset > data->block_size && i < block_count) { - off += SQFS_ON_DISK_BLOCK_SIZE(inode->extra[i++]); - offset -= data->block_size; + if ((filesz - offset) < (sqfs_u64)size) + size = filesz - offset; - if (filesz >= data->block_size) { - filesz -= data->block_size; - } else { - filesz = 0; - } + if (size == 0) + return 0; + + /* find location of the first block */ + for (i = 0; offset > data->block_size && i < block_count; ++i) { + off += SQFS_ON_DISK_BLOCK_SIZE(inode->extra[i]); + offset -= data->block_size; } /* copy data from blocks */ - while (i < block_count && size > 0 && filesz > 0) { + while (i < block_count && size > 0) { diff = data->block_size - offset; if (size < diff) diff = size; @@ -339,12 +340,6 @@ sqfs_s32 sqfs_data_reader_read(sqfs_data_reader_t *data, off += SQFS_ON_DISK_BLOCK_SIZE(inode->extra[i]); } - if (filesz >= data->block_size) { - filesz -= data->block_size; - } else { - filesz = 0; - } - ++i; offset = 0; size -= diff; @@ -353,22 +348,16 @@ sqfs_s32 sqfs_data_reader_read(sqfs_data_reader_t *data, } /* copy from fragment */ - if (i == block_count && size > 0 && filesz > 0) { + if (size > 0) { err = precache_fragment_block(data, frag_idx); if (err) return err; - if (frag_off + filesz > data->block_size) + if ((frag_off + offset) >= data->frag_blk_size) return SQFS_ERROR_OUT_OF_BOUNDS; - if (offset >= filesz) - return total; - - if (offset + size > filesz) - size = filesz - offset; - - if (size == 0) - return total; + if ((data->frag_blk_size - (frag_off + offset)) < size) + return SQFS_ERROR_OUT_OF_BOUNDS; ptr = (char *)data->frag_block + frag_off + offset; memcpy(buffer, ptr, size); -- cgit v1.2.3