qcow: bounds check the refcount table offset and size

If the header puts the refcount table outside the file size or if it
specifies a table much larger than needed, fail to open the file.

These might not be hard qcow errors, but they are situations that crosvm
will never encounter.

BUG=986061
TEST=fuzzer with new test cases completes in less than 5 seconds.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Change-Id: If048c96f6255ca81740e20f3f4eb7669467dbb7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1716365
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>

1 files changed, 33 insertions, 2 deletions

diff --git a/qcow/src/qcow.rs b/qcow/src/qcow.rs
index e2d391b..73d750f 100644
--- a/qcow/src/qcow.rs
+++ b/qcow/src/qcow.rs
@@ -14,7 +14,7 @@ use sys_util::{
     error, FileReadWriteVolatile, FileSetLen, FileSync, PunchHole, SeekHole, WriteZeroes,
 };
 
-use std::cmp::min;
+use std::cmp::{max, min};
 use std::fmt::{self, Display};
 use std::fs::File;
 use std::io::{self, Read, Seek, SeekFrom, Write};
@@ -53,6 +53,8 @@ pub enum Error {
     ReadingRefCountBlock(refcount::Error),
     ReadingRefCounts(io::Error),
     RebuildingRefCounts(io::Error),
+    RefcountTableOffEnd,
+    RefcountTableTooLarge,
     SeekingFile(io::Error),
     SettingFileSize(io::Error),
     SettingRefcountRefcount(io::Error),
@@ -103,6 +105,8 @@ impl Display for Error {
             ReadingRefCountBlock(e) => write!(f, "failed to read ref count block: {}", e),
             ReadingRefCounts(e) => write!(f, "failed to read ref counts: {}", e),
             RebuildingRefCounts(e) => write!(f, "failed to rebuild ref counts: {}", e),
+            RefcountTableOffEnd => write!(f, "refcount table offset past file end"),
+            RefcountTableTooLarge => write!(f, "too many clusters specified for refcount table"),
             SeekingFile(e) => write!(f, "failed to seek file: {}", e),
             SettingFileSize(e) => write!(f, "failed to set file size: {}", e),
             SettingRefcountRefcount(e) => write!(f, "failed to set refcount refcount: {}", e),
@@ -402,8 +406,13 @@ impl QcowFile {
         }
         offset_is_cluster_boundary(header.backing_file_offset, header.cluster_bits)?;
         offset_is_cluster_boundary(header.l1_table_offset, header.cluster_bits)?;
-        offset_is_cluster_boundary(header.refcount_table_offset, header.cluster_bits)?;
         offset_is_cluster_boundary(header.snapshots_offset, header.cluster_bits)?;
+        // refcount table must be a cluster boundary, and within the file's virtual or actual size.
+        offset_is_cluster_boundary(header.refcount_table_offset, header.cluster_bits)?;
+        let file_size = file.metadata().map_err(Error::GettingFileSize)?.len();
+        if header.refcount_table_offset > max(file_size, header.size) {
+            return Err(Error::RefcountTableOffEnd);
+        }
 
         // The first cluster should always have a non-zero refcount, so if it is 0,
         // this is an old file with broken refcounts, which requires a rebuild.
@@ -455,6 +464,10 @@ impl QcowFile {
             cluster_size as u32,
             (num_clusters + l1_clusters + num_l2_clusters + header_clusters) as u32,
         );
+        // Check that the given header doesn't have a suspiciously sized refcount table.
+        if u64::from(header.refcount_table_clusters) > 2 * refcount_clusters {
+            return Err(Error::RefcountTableTooLarge);
+        }
         if l1_clusters + refcount_clusters > MAX_RAM_POINTER_TABLE_SIZE {
             return Err(Error::TooManyRefcounts(refcount_clusters));
         }
@@ -1873,6 +1886,24 @@ mod tests {
     }
 
     #[test]
+    fn test_header_huge_num_refcounts() {
+        let mut header = valid_header();
+        &mut header[56..60].copy_from_slice(&[0x02, 0x00, 0xe8, 0xff]);
+        with_basic_file(&header, |disk_file: File| {
+            QcowFile::from(disk_file).expect_err("Created disk with crazy refcount clusters");
+        });
+    }
+
+    #[test]
+    fn test_header_huge_refcount_offset() {
+        let mut header = valid_header();
+        &mut header[48..56].copy_from_slice(&[0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x02, 0x00]);
+        with_basic_file(&header, |disk_file: File| {
+            QcowFile::from(disk_file).expect_err("Created disk with crazy refcount offset");
+        });
+    }
+
+    #[test]
     fn write_read_start() {
         with_basic_file(&valid_header(), |disk_file: File| {
             let mut q = QcowFile::from(disk_file).unwrap();