From 3cc70889a35a972358e9b0e768a06b7159def1f3 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Sun, 24 Mar 2024 19:17:01 +0000 Subject: [PATCH] esp.c: prevent cmdfifo overflow in esp_cdb_ready() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During normal use the cmdfifo will never wrap internally and cmdfifo_cdb_offset will always indicate the start of the SCSI CDB. However it is possible that a malicious guest could issue an invalid ESP command sequence such that cmdfifo wraps internally and cmdfifo_cdb_offset could point beyond the end of the FIFO data buffer. Add an extra check to fifo8_peek_buf() to ensure that if the cmdfifo has wrapped internally then esp_cdb_ready() will exit rather than allow scsi_cdb_length() to access data outside the cmdfifo data buffer. Reported-by: Chuhong Yuan Signed-off-by: Mark Cave-Ayland Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20240324191707.623175-13-mark.cave-ayland@ilande.co.uk> Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index f47abc36d6..d8db33b921 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -429,13 +429,23 @@ static bool esp_cdb_ready(ESPState *s) { int len = fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset; const uint8_t *pbuf; + uint32_t n; int cdblen; if (len <= 0) { return false; } - pbuf = fifo8_peek_buf(&s->cmdfifo, len, NULL); + pbuf = fifo8_peek_buf(&s->cmdfifo, len, &n); + if (n < len) { + /* + * In normal use the cmdfifo should never wrap, but include this check + * to prevent a malicious guest from reading past the end of the + * cmdfifo data buffer below + */ + return false; + } + cdblen = scsi_cdb_length((uint8_t *)&pbuf[s->cmdfifo_cdb_offset]); return cdblen < 0 ? false : (len >= cdblen);