From 73080d3a53ecc15bd3e0967857fc7093a10d2c8e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Klaus=20K=C3=A4mpf?= <kkaempf@gmail.com>
Date: Tue, 2 Apr 2024 18:57:29 +0200
Subject: [PATCH] Improve buffer overflow checking in
 scsi_command_util::ModeSelect
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
---
 cpp/devices/scsi_command_util.cpp | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/cpp/devices/scsi_command_util.cpp b/cpp/devices/scsi_command_util.cpp
index f7b31d22..940be2bd 100644
--- a/cpp/devices/scsi_command_util.cpp
+++ b/cpp/devices/scsi_command_util.cpp
@@ -33,9 +33,17 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
 	// Skip block descriptors
 	int offset;
 	if (cmd == scsi_command::eCmdModeSelect10) {
+        	if (length < 7) {
+			spdlog::warn("Incomplete Mode parameter header(10)");
+			throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_parameter_list);
+		}
 		offset = 8 + GetInt16(buf, 6);
 	}
 	else {
+        	if (length < 4) {
+			spdlog::warn("Incomplete Mode parameter header(6)");
+			throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_parameter_list);
+		}
 		offset = 4 + buf[3];
 	}
 	length -= offset;
@@ -44,8 +52,7 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
 	bool has_valid_page_code = (length == 0);
 
 	// Parse the pages
-        // expect (remaining) length > 1 because we access buf[offset+1] below
-	while (length > 1) {
+	while (length > 0) {
 		// Format device page
 		if (const int page = buf[offset]; page == 0x03) {
 			if (length < 14) {
@@ -68,12 +75,20 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
 			// OpenVMS Alpha 7.3 uses this
 			has_valid_page_code = true;
 		}
+		else if ((page == 0x00) && (length == 1)) {
+			has_valid_page_code = true;
+			break; // page 0 is valid if last in list, might have no size byte following
+		}
 		else {
 			stringstream s;
 			s << "Unknown MODE SELECT page code: $" << setfill('0') << setw(2) << hex << page;
 			result = s.str();
 		}
 
+		if (length < 2) { // ensure there's a size byte before accessing it
+                    spdlog::warn("Current MODE SELECT page has no size");
+			throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_parameter_list);
+		}
 		// Advance to the next page
 		const int size = buf[offset + 1] + 2;