mirror of
https://chromium.googlesource.com/crosvm/crosvm
synced 2025-02-06 02:25:23 +00:00
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>
This commit is contained in:
parent
a08e40bf81
commit
969a0b49ff
1 changed files with 33 additions and 2 deletions
|
@ -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| {
|
||||
|
|
Loading…
Reference in a new issue