From 09fe1fd2a1f9efc3091b4fc61f1876d0785956a8 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Sun, 1 Sep 2019 00:59:16 -0400 Subject: libsupport: add checks to prevent buffer overrun bugs in quota code A maliciously corrupted file systems can trigger buffer overruns in the quota code used by e2fsck. To fix this, add sanity checks to the quota header fields as well as to block number references in the quota tree. Addresses: CVE-2019-5094 Addresses: TALOS-2019-0887 Signed-off-by: Theodore Ts'o (cherry picked from commit 8dbe7b475ec5e91ed767239f0e85880f416fc384) --- lib/support/mkquota.c | 1 + lib/support/quotaio_tree.c | 71 ++++++++++++++++++++++++++++++---------------- lib/support/quotaio_v2.c | 28 ++++++++++++++++++ 3 files changed, 76 insertions(+), 24 deletions(-) --- a/lib/support/mkquota.c +++ b/lib/support/mkquota.c @@ -671,6 +671,7 @@ errcode_t quota_compare_and_update(quota err = qh.qh_ops->scan_dquots(&qh, scan_dquots_callback, &scan_data); if (err) { log_debug("Error scanning dquots"); + *usage_inconsistent = 1; goto out_close_qh; } --- a/lib/support/quotaio_tree.c +++ b/lib/support/quotaio_tree.c @@ -540,6 +540,17 @@ struct dquot *qtree_read_dquot(struct qu return dquot; } +static int check_reference(struct quota_handle *h, unsigned int blk) +{ + if (blk >= h->qh_info.u.v2_mdqi.dqi_qtree.dqi_blocks) { + log_err("Illegal reference (%u >= %u) in %s quota file", + blk, h->qh_info.u.v2_mdqi.dqi_qtree.dqi_blocks, + quota_type2name(h->qh_type)); + return -1; + } + return 0; +} + /* * Scan all dquots in file and call callback on each */ @@ -558,7 +569,7 @@ static int report_block(struct dquot *dq int entries, i; if (!buf) - return 0; + return -1; set_bit(bitmap, blk); read_blk(dquot->dq_h, blk, buf); @@ -580,23 +591,12 @@ static int report_block(struct dquot *dq return entries; } -static void check_reference(struct quota_handle *h, unsigned int blk) -{ - if (blk >= h->qh_info.u.v2_mdqi.dqi_qtree.dqi_blocks) - log_err("Illegal reference (%u >= %u) in %s quota file. " - "Quota file is probably corrupted.\n" - "Please run e2fsck (8) to fix it.", - blk, - h->qh_info.u.v2_mdqi.dqi_qtree.dqi_blocks, - quota_type2name(h->qh_type)); -} - static int report_tree(struct dquot *dquot, unsigned int blk, int depth, char *bitmap, int (*process_dquot) (struct dquot *, void *), void *data) { - int entries = 0, i; + int entries = 0, ret, i; dqbuf_t buf = getdqbuf(); __le32 *ref = (__le32 *) buf; @@ -607,22 +607,40 @@ static int report_tree(struct dquot *dqu if (depth == QT_TREEDEPTH - 1) { for (i = 0; i < QT_BLKSIZE >> 2; i++) { blk = ext2fs_le32_to_cpu(ref[i]); - check_reference(dquot->dq_h, blk); - if (blk && !get_bit(bitmap, blk)) - entries += report_block(dquot, blk, bitmap, - process_dquot, data); + if (check_reference(dquot->dq_h, blk)) { + entries = -1; + goto errout; + } + if (blk && !get_bit(bitmap, blk)) { + ret = report_block(dquot, blk, bitmap, + process_dquot, data); + if (ret < 0) { + entries = ret; + goto errout; + } + entries += ret; + } } } else { for (i = 0; i < QT_BLKSIZE >> 2; i++) { blk = ext2fs_le32_to_cpu(ref[i]); if (blk) { - check_reference(dquot->dq_h, blk); - entries += report_tree(dquot, blk, depth + 1, - bitmap, process_dquot, - data); + if (check_reference(dquot->dq_h, blk)) { + entries = -1; + goto errout; + } + ret = report_tree(dquot, blk, depth + 1, + bitmap, process_dquot, + data); + if (ret < 0) { + entries = ret; + goto errout; + } + entries += ret; } } } +errout: freedqbuf(buf); return entries; } @@ -642,6 +660,7 @@ int qtree_scan_dquots(struct quota_handl int (*process_dquot) (struct dquot *, void *), void *data) { + int ret; char *bitmap; struct v2_mem_dqinfo *v2info = &h->qh_info.u.v2_mdqi; struct qtree_mem_dqinfo *info = &v2info->dqi_qtree; @@ -655,10 +674,14 @@ int qtree_scan_dquots(struct quota_handl ext2fs_free_mem(&dquot); return -1; } - v2info->dqi_used_entries = report_tree(dquot, QT_TREEOFF, 0, bitmap, - process_dquot, data); + ret = report_tree(dquot, QT_TREEOFF, 0, bitmap, process_dquot, data); + if (ret < 0) + goto errout; + v2info->dqi_used_entries = ret; v2info->dqi_data_blocks = find_set_bits(bitmap, info->dqi_blocks); + ret = 0; +errout: ext2fs_free_mem(&bitmap); ext2fs_free_mem(&dquot); - return 0; + return ret; } --- a/lib/support/quotaio_v2.c +++ b/lib/support/quotaio_v2.c @@ -175,6 +175,8 @@ static int v2_check_file(struct quota_ha static int v2_init_io(struct quota_handle *h) { struct v2_disk_dqinfo ddqinfo; + struct v2_mem_dqinfo *info; + __u64 filesize; h->qh_info.u.v2_mdqi.dqi_qtree.dqi_entry_size = sizeof(struct v2r1_disk_dqblk); @@ -185,6 +187,32 @@ static int v2_init_io(struct quota_handl sizeof(ddqinfo)) != sizeof(ddqinfo)) return -1; v2_disk2memdqinfo(&h->qh_info, &ddqinfo); + + /* Check to make sure quota file info is sane */ + info = &h->qh_info.u.v2_mdqi; + if (ext2fs_file_get_lsize(h->qh_qf.e2_file, &filesize)) + return -1; + if ((filesize > (1U << 31)) || + (info->dqi_qtree.dqi_blocks > + (filesize + QT_BLKSIZE - 1) >> QT_BLKSIZE_BITS)) { + log_err("Quota inode %u corrupted: file size %llu; " + "dqi_blocks %u", h->qh_qf.ino, + filesize, info->dqi_qtree.dqi_blocks); + return -1; + } + if (info->dqi_qtree.dqi_free_blk >= info->dqi_qtree.dqi_blocks) { + log_err("Quota inode %u corrupted: free_blk %u; dqi_blocks %u", + h->qh_qf.ino, info->dqi_qtree.dqi_free_blk, + info->dqi_qtree.dqi_blocks); + return -1; + } + if (info->dqi_qtree.dqi_free_entry >= info->dqi_qtree.dqi_blocks) { + log_err("Quota inode %u corrupted: free_entry %u; " + "dqi_blocks %u", h->qh_qf.ino, + info->dqi_qtree.dqi_free_entry, + info->dqi_qtree.dqi_blocks); + return -1; + } return 0; }