From 9ae7d6b38a75e09b7ca5684a83ef12338df789de Mon Sep 17 00:00:00 2001 From: MattJackson <1085847+MattJackson@users.noreply.github.com> Date: Wed, 15 Apr 2026 04:29:45 +0000 Subject: [PATCH] Fix audit findings: SCSI constants, sg_io_hdr assert, handshake cap, sector overflow check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace magic SCSI opcodes with named constants (S2) - Add compile-time sg_io_hdr size assertion — 88 bytes on 64-bit (W2) - Cap handshake cert attempts at 16 (W8) - Validate IsoSectorReader/FileSectorReader against u32 overflow for >8TB (S8) - encrypt.rs: limit host cert loop iterations --- src/disc/encrypt.rs | 3 ++- src/drive/mod.rs | 32 ++++++++++++++++++++------------ src/mux/iso.rs | 9 ++++++++- src/scsi/linux.rs | 7 +++++++ src/sector.rs | 9 ++++++++- 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/disc/encrypt.rs b/src/disc/encrypt.rs index 22724b9..d865849 100644 --- a/src/disc/encrypt.rs +++ b/src/disc/encrypt.rs @@ -25,7 +25,8 @@ impl Disc { let keydb_path = opts.resolve_keydb()?; let keydb = KeyDb::load(&keydb_path).ok()?; - for hc in &keydb.host_certs { + const MAX_CERT_ATTEMPTS: usize = 16; + for hc in keydb.host_certs.iter().take(MAX_CERT_ATTEMPTS) { match aacs::handshake::aacs_authenticate(session, &hc.private_key, &hc.certificate) { Ok(mut auth) => { let volume_id = match aacs::handshake::read_volume_id(session, &mut auth) { diff --git a/src/drive/mod.rs b/src/drive/mod.rs index 2b623c2..f780134 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -39,6 +39,14 @@ pub enum DriveStatus { Unknown, } +// SCSI opcodes used in drive control +const SCSI_TEST_UNIT_READY: u8 = 0x00; +const SCSI_START_STOP_UNIT: u8 = 0x1B; +const SCSI_PREVENT_ALLOW_MEDIUM_REMOVAL: u8 = 0x1E; +const SCSI_GET_EVENT_STATUS: u8 = 0x4A; +const SCSI_MODE_SENSE: u8 = 0x5A; +const SCSI_REPORT_KEY: u8 = 0xA4; + /// Recovery state after a read error — stay at min speed for N bytes. const RECOVERY_WINDOW: u64 = 500 * 1024 * 1024; // 500 MB @@ -104,7 +112,7 @@ impl Drive { } pub fn wait_ready(&mut self) -> Result<()> { - let tur = [0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; + let tur = [SCSI_TEST_UNIT_READY, 0x00, 0x00, 0x00, 0x00, 0x00]; let mut tried_reset = false; for _ in 0..60 { @@ -142,7 +150,7 @@ impl Drive { /// Uses GET EVENT STATUS NOTIFICATION which works regardless of firmware state. pub fn drive_status(&mut self) -> DriveStatus { // GET EVENT STATUS NOTIFICATION: polled, media event class (0x10) - let cdb = [0x4Au8, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x08, 0x00]; + let cdb = [SCSI_GET_EVENT_STATUS, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x08, 0x00]; let mut buf = [0u8; 8]; match self.scsi.as_mut().execute( &cdb, @@ -164,7 +172,7 @@ impl Drive { } _ => { // Fallback: try TUR - let tur = [0x00u8, 0x00, 0x00, 0x00, 0x00, 0x00]; + let tur = [SCSI_TEST_UNIT_READY, 0x00, 0x00, 0x00, 0x00, 0x00]; let mut empty = [0u8; 0]; match self.scsi.as_mut().execute( &tur, @@ -193,17 +201,17 @@ impl Drive { /// any step, even if the drive reports "tray open" (that's a valid state). pub fn reset(&mut self) -> Result<()> { let mut buf = [0u8; 0]; - let tur = [0x00u8, 0x00, 0x00, 0x00, 0x00, 0x00]; + let tur = [SCSI_TEST_UNIT_READY, 0x00, 0x00, 0x00, 0x00, 0x00]; // 1. Unlock + stop/start self.unlock_tray(); - let stop = [0x1Bu8, 0x00, 0x00, 0x00, 0x00, 0x00]; + let stop = [SCSI_START_STOP_UNIT, 0x00, 0x00, 0x00, 0x00, 0x00]; let _ = self.scsi .as_mut() .execute(&stop, crate::scsi::DataDirection::None, &mut buf, 5_000); std::thread::sleep(std::time::Duration::from_millis(500)); - let start = [0x1Bu8, 0x00, 0x00, 0x00, 0x01, 0x00]; + let start = [SCSI_START_STOP_UNIT, 0x00, 0x00, 0x00, 0x01, 0x00]; let _ = self.scsi .as_mut() @@ -223,7 +231,7 @@ impl Drive { // After eject, TUR returning "Not Ready — tray open" (sense key 2) // counts as success: the drive is functional, just needs disc reinserted. self.unlock_tray(); - let eject = [0x1Bu8, 0x00, 0x00, 0x00, 0x02, 0x00]; + let eject = [SCSI_START_STOP_UNIT, 0x00, 0x00, 0x00, 0x02, 0x00]; let _ = self.scsi .as_mut() @@ -335,7 +343,7 @@ impl Drive { /// Read REPORT KEY RPC state (region playback control). pub fn report_key_rpc_state(&mut self) -> Option> { let cdb = [ - 0xA4u8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x08, 0x00, + SCSI_REPORT_KEY, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x08, 0x00, ]; let mut buf = vec![0u8; 8]; let r = self @@ -357,7 +365,7 @@ impl Drive { /// Read MODE SENSE page data. pub fn mode_sense_page(&mut self, page: u8) -> Option> { - let cdb = [0x5Au8, 0x00, page, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFC, 0x00]; + let cdb = [SCSI_MODE_SENSE, 0x00, page, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFC, 0x00]; let mut buf = vec![0u8; 252]; let r = self .scsi @@ -516,7 +524,7 @@ impl Drive { /// Lock the tray so the disc cannot be ejected during a rip. pub fn lock_tray(&mut self) { - let prevent = [0x1Eu8, 0x00, 0x00, 0x00, 0x01, 0x00]; + let prevent = [SCSI_PREVENT_ALLOW_MEDIUM_REMOVAL, 0x00, 0x00, 0x00, 0x01, 0x00]; let mut buf = [0u8; 0]; let _ = self.scsi @@ -526,7 +534,7 @@ impl Drive { /// Unlock the tray so the user can manually eject the disc. pub fn unlock_tray(&mut self) { - let allow = [0x1Eu8, 0x00, 0x00, 0x00, 0x00, 0x00]; + let allow = [SCSI_PREVENT_ALLOW_MEDIUM_REMOVAL, 0x00, 0x00, 0x00, 0x00, 0x00]; let mut buf = [0u8; 0]; let _ = self.scsi @@ -537,7 +545,7 @@ impl Drive { /// Eject the disc tray. Unlocks first, then ejects. pub fn eject(&mut self) -> Result<()> { self.unlock_tray(); - let eject_cdb = [0x1Bu8, 0, 0, 0, 0x02, 0]; + let eject_cdb = [SCSI_START_STOP_UNIT, 0, 0, 0, 0x02, 0]; let mut buf = [0u8; 0]; self.scsi.as_mut().execute( &eject_cdb, diff --git a/src/mux/iso.rs b/src/mux/iso.rs index 6d38d4c..164e867 100644 --- a/src/mux/iso.rs +++ b/src/mux/iso.rs @@ -33,7 +33,14 @@ impl IsoSectorReader { let file = File::open(Path::new(path)) .map_err(|e| io::Error::new(e.kind(), format!("iso://{path}: {e}")))?; let size = file.metadata()?.len(); - let capacity = (size / SECTOR_SIZE) as u32; + let sectors = size / SECTOR_SIZE; + if sectors > u32::MAX as u64 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("iso://{path}: image too large ({} TB, max ~8 TB)", size / (1024 * 1024 * 1024 * 1024)), + )); + } + let capacity = sectors as u32; Ok(Self { file, capacity }) } diff --git a/src/scsi/linux.rs b/src/scsi/linux.rs index b42e0d4..8d2371e 100644 --- a/src/scsi/linux.rs +++ b/src/scsi/linux.rs @@ -40,6 +40,13 @@ struct sg_io_hdr { info: u32, } +// Compile-time validation: sg_io_hdr must match the kernel's layout. +// 64 bytes on 64-bit, 44 bytes on 32-bit (pointer-size dependent). +#[cfg(target_pointer_width = "64")] +const _: () = assert!(std::mem::size_of::() == 88); +#[cfg(target_pointer_width = "32")] +const _: () = assert!(std::mem::size_of::() == 64); + pub struct SgIoTransport { fd: i32, } diff --git a/src/sector.rs b/src/sector.rs index c65ed5e..cf3274e 100644 --- a/src/sector.rs +++ b/src/sector.rs @@ -27,7 +27,14 @@ impl FileSectorReader { pub fn open(path: &str) -> std::io::Result { let file = std::fs::File::open(path)?; let len = file.metadata()?.len(); - let capacity = (len / 2048) as u32; + let sectors = len / 2048; + if sectors > u32::MAX as u64 { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("{path}: image too large, max ~8 TB"), + )); + } + let capacity = sectors as u32; Ok(Self { file: std::io::BufReader::with_capacity(4 * 1024 * 1024, file), capacity,