[U-Boot] [PATCH 1/5] CVE-2019-13103: disk: stop infinite recursion in DOS Partitions
part_get_info_extended and print_partition_extended can recurse infinitely while parsing a self-referential filesystem or one with a silly number of extended partitions. This patch adds a limit to the number of recursive partitions.
Signed-off-by: Paul Emge paulemge@forallsecure.com --- disk/part_dos.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/disk/part_dos.c b/disk/part_dos.c index 936cee0d36..aae9d95906 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -23,6 +23,10 @@
#define DOS_PART_DEFAULT_SECTOR 512
+/* should this be configurable? It looks like it's not very common at all + * to use large numbers of partitions */ +#define MAX_EXT_PARTS 256 + /* Convert char[4] in little endian format to the host format integer */ static inline unsigned int le32_to_int(unsigned char *le32) @@ -126,6 +130,13 @@ static void print_partition_extended(struct blk_desc *dev_desc, dos_partition_t *pt; int i;
+ /* set a maximum recursion level */ + if (part_num > MAX_EXT_PARTS) + { + printf("** Nested DOS partitions detected, stopping **\n"); + return; + } + if (blk_dread(dev_desc, ext_part_sector, 1, (ulong *)buffer) != 1) { printf ("** Can't read partition table on %d:" LBAFU " **\n", dev_desc->devnum, ext_part_sector); @@ -191,6 +202,13 @@ static int part_get_info_extended(struct blk_desc *dev_desc, int i; int dos_type;
+ /* set a maximum recursion level */ + if (part_num > MAX_EXT_PARTS) + { + printf("** Nested DOS partitions detected, stopping **\n"); + return -1; + } + if (blk_dread(dev_desc, ext_part_sector, 1, (ulong *)buffer) != 1) { printf ("** Can't read partition table on %d:" LBAFU " **\n", dev_desc->devnum, ext_part_sector);
ext_cache_read doesn't null cache->buf, after freeing, which results in a later function double-freeing it. This patch fixes ext_cache_read to call ext_cache_fini instead of free.
Signed-off-by: Paul Emge paulemge@forallsecure.com --- fs/ext4/ext4fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 26db677a1f..85dc122f30 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -286,7 +286,7 @@ int ext_cache_read(struct ext_block_cache *cache, lbaint_t block, int size) if (!cache->buf) return 0; if (!ext4fs_devread(block, 0, size, cache->buf)) { - free(cache->buf); + ext_cache_fini(cache); return 0; } cache->block = block;
On Mon, Jul 08, 2019 at 04:37:04PM -0700, Paul Emge wrote:
ext_cache_read doesn't null cache->buf, after freeing, which results in a later function double-freeing it. This patch fixes ext_cache_read to call ext_cache_fini instead of free.
Signed-off-by: Paul Emge paulemge@forallsecure.com
Applied to u-boot/master, thanks!
in ext4fs_read_file, it is possible for a broken/malicious file system to cause a memcpy of a negative number of bytes, which overflows all memory. This patch fixes the issue by checking for a negative length.
Signed-off-by: Paul Emge paulemge@forallsecure.com --- fs/ext4/ext4fs.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 85dc122f30..e2b740cac4 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -66,13 +66,15 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos,
ext_cache_init(&cache);
- if (blocksize <= 0) - return -1; - /* Adjust len so it we can't read past the end of the file. */ if (len + pos > filesize) len = (filesize - pos);
+ if (blocksize <= 0 || len <= 0) { + ext_cache_fini(&cache); + return -1; + } + blockcnt = lldiv(((len + pos) + blocksize - 1), blocksize);
for (i = lldiv(pos, blocksize); i < blockcnt; i++) {
On Mon, Jul 08, 2019 at 04:37:05PM -0700, Paul Emge wrote:
in ext4fs_read_file, it is possible for a broken/malicious file system to cause a memcpy of a negative number of bytes, which overflows all memory. This patch fixes the issue by checking for a negative length.
Signed-off-by: Paul Emge paulemge@forallsecure.com
Applied to u-boot/master, thanks!
This patch checks for 0 in several ext4 headers and gracefully fails instead of raising a divide-by-0 exception.
Signed-off-by: Paul Emge paulemge@forallsecure.com --- fs/ext4/ext4_common.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index 464c33d0d7..0d15e0c3cd 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -1571,8 +1571,12 @@ static int ext4fs_blockgroup int log2blksz = get_fs()->dev_desc->log2blksz; int desc_size = get_fs()->gdsize;
+ if (desc_size == 0) + return 0; desc_per_blk = EXT2_BLOCK_SIZE(data) / desc_size;
+ if (desc_per_blk == 0) + return 0; blkno = le32_to_cpu(data->sblock.first_data_block) + 1 + group / desc_per_blk; blkoff = (group % desc_per_blk) * desc_size; @@ -1602,6 +1606,10 @@ int ext4fs_read_inode(struct ext2_data *data, int ino, struct ext2_inode *inode)
/* It is easier to calculate if the first inode is 0. */ ino--; + if ( le32_to_cpu(sblock->inodes_per_group) == 0 || fs->inodesz == 0) { + free(blkgrp); + return 0; + } status = ext4fs_blockgroup(data, ino / le32_to_cpu (sblock->inodes_per_group), blkgrp); if (status == 0) { @@ -1610,6 +1618,10 @@ int ext4fs_read_inode(struct ext2_data *data, int ino, struct ext2_inode *inode) }
inodes_per_block = EXT2_BLOCK_SIZE(data) / fs->inodesz; + if ( inodes_per_block == 0 ) { + free(blkgrp); + return 0; + } blkno = ext4fs_bg_get_inode_table_id(blkgrp, fs) + (ino % le32_to_cpu(sblock->inodes_per_group)) / inodes_per_block; blkoff = (ino % inodes_per_block) * fs->inodesz;
On Mon, Jul 08, 2019 at 04:37:06PM -0700, Paul Emge wrote:
This patch checks for 0 in several ext4 headers and gracefully fails instead of raising a divide-by-0 exception.
Signed-off-by: Paul Emge paulemge@forallsecure.com
Applied to u-boot/master, thanks!
In ext4fs_read_file in ext4fs.c, a memset can overwrite the bounds of the destination memory region. This patch adds a check to disallow this.
Signed-off-by: Paul Emge paulemge@forallsecure.com --- fs/ext4/ext4fs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index e2b740cac4..37b31d9f0f 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -61,6 +61,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, lbaint_t delayed_skipfirst = 0; lbaint_t delayed_next = 0; char *delayed_buf = NULL; + char *start_buf = buf; short status; struct ext_block_cache cache;
@@ -139,6 +140,7 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, } } else { int n; + int n_left; if (previous_block_number != -1) { /* spill */ status = ext4fs_devread(delayed_start, @@ -153,8 +155,9 @@ int ext4fs_read_file(struct ext2fs_node *node, loff_t pos, } /* Zero no more than `len' bytes. */ n = blocksize - skipfirst; - if (n > len) - n = len; + n_left = len - ( buf - start_buf ); + if (n > n_left) + n = n_left; memset(buf, 0, n); } buf += blocksize - skipfirst;
On Mon, Jul 08, 2019 at 04:37:07PM -0700, Paul Emge wrote:
In ext4fs_read_file in ext4fs.c, a memset can overwrite the bounds of the destination memory region. This patch adds a check to disallow this.
Signed-off-by: Paul Emge paulemge@forallsecure.com
Applied to u-boot/master, thanks!
On Mon, Jul 08, 2019 at 04:37:03PM -0700, Paul Emge wrote:
part_get_info_extended and print_partition_extended can recurse infinitely while parsing a self-referential filesystem or one with a silly number of extended partitions. This patch adds a limit to the number of recursive partitions.
Signed-off-by: Paul Emge paulemge@forallsecure.com
Applied to u-boot/master, thanks!
participants (2)
-
Paul Emge -
Tom Rini