From 969a0b49ff0a9afbca18230181542bbe7e06b8f7 Mon Sep 17 00:00:00 2001 From: Dylan Reid Date: Tue, 23 Jul 2019 19:51:33 -0700 Subject: 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 Change-Id: If048c96f6255ca81740e20f3f4eb7669467dbb7b Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1716365 Reviewed-by: Daniel Verkamp Tested-by: kokoro --- qcow/src/qcow.rs | 35 +++++++++++++++++++++++++++++++++-- 1 file 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)); } @@ -1872,6 +1885,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| { -- cgit 1.4.1