[Qemu-devel] [PATCH 0/2] Qemu: scsi: esp: check command buffer input length

[Qemu-devel] [PATCH 0/2] Qemu: scsi: esp: check command buffer input length

[P J P]

From: Prasad J Pandit <pjp@fedoraproject.org>

Hello,

The ESP 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte
FIFO buffer. It is used to handle command and data transfer between
controller and the bus. Couple of OOB write access issues were found
and reported in its emulation by Mr Li Qiang of 360.cn Inc.

Please see below are the proposed patches to fix these issues.

Thank you.
--
Prasad J Pandit (2):
  scsi: check command buffer length before write(CVE-2016-4439)
  scsi: check dma length before reading scsi command(CVE-2016-4441)

 hw/scsi/esp.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

--
2.5.5

[Qemu-devel] [PATCH 1/2] scsi: check command buffer length before write(CVE-2016-4439)

[P J P]

From: Prasad J Pandit <pjp@fedoraproject.org>

The 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte
FIFO buffer. It is used to handle command and data transfer. While
writing to this command buffer 's->cmdbuf[TI_BUFSZ=16]', a check
was missing to validate input length. Add check to avoid OOB write
access.

Fixes CVE-2016-4439
Reported-by: Li Qiang <liqiang6-s@360.cn>

Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/scsi/esp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8961be2..01497e6 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -448,7 +448,11 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val)
         break;
     case ESP_FIFO:
         if (s->do_cmd) {
-            s->cmdbuf[s->cmdlen++] = val & 0xff;
+            if (s->cmdlen < TI_BUFSZ) {
+                s->cmdbuf[s->cmdlen++] = val & 0xff;
+            } else {
+                trace_esp_error_fifo_overrun();
+            }
         } else if (s->ti_size == TI_BUFSZ - 1) {
             trace_esp_error_fifo_overrun();
         } else {
-- 
2.5.5

[Qemu-devel] [PATCH 2/2] scsi: check dma length before reading scsi command(CVE-2016-4441)

[P J P]

From: Prasad J Pandit <pjp@fedoraproject.org>

The 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte
FIFO buffer. It is used to handle command and data transfer.
Routine get_cmd() uses DMA to read scsi commands into this buffer.
Add check to validate DMA length against buffer size to avoid any
overrun.

Fixes CVE-2016-4441
Reported-by: Li Qiang <liqiang6-s@360.cn>

Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/scsi/esp.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 01497e6..591c817 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -82,7 +82,7 @@ void esp_request_cancelled(SCSIRequest *req)
     }
 }
 
-static uint32_t get_cmd(ESPState *s, uint8_t *buf)
+static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
 {
     uint32_t dmalen;
     int target;
@@ -92,6 +92,9 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
         dmalen = s->rregs[ESP_TCLO];
         dmalen |= s->rregs[ESP_TCMID] << 8;
         dmalen |= s->rregs[ESP_TCHI] << 16;
+        if (dmalen > buflen) {
+            return 0;
+        }
         s->dma_memory_read(s->dma_opaque, buf, dmalen);
     } else {
         dmalen = s->ti_size;
@@ -166,7 +169,7 @@ static void handle_satn(ESPState *s)
         s->dma_cb = handle_satn;
         return;
     }
-    len = get_cmd(s, buf);
+    len = get_cmd(s, buf, sizeof(buf));
     if (len)
         do_cmd(s, buf);
 }
@@ -180,7 +183,7 @@ static void handle_s_without_atn(ESPState *s)
         s->dma_cb = handle_s_without_atn;
         return;
     }
-    len = get_cmd(s, buf);
+    len = get_cmd(s, buf, sizeof(buf));
     if (len) {
         do_busid_cmd(s, buf, 0);
     }
@@ -192,7 +195,7 @@ static void handle_satn_stop(ESPState *s)
         s->dma_cb = handle_satn_stop;
         return;
     }
-    s->cmdlen = get_cmd(s, s->cmdbuf);
+    s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
     if (s->cmdlen) {
         trace_esp_handle_satn_stop(s->cmdlen);
         s->do_cmd = 1;
-- 
2.5.5

Re: [Qemu-devel] [PATCH 0/2] Qemu: scsi: esp: check command buffer input length

[Paolo Bonzini]

On 19/05/2016 12:39, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> Hello,
> 
> The ESP 53C9X Fast SCSI Controller(FSC) comes with an internal 16-byte
> FIFO buffer. It is used to handle command and data transfer between
> controller and the bus. Couple of OOB write access issues were found
> and reported in its emulation by Mr Li Qiang of 360.cn Inc.
> 
> Please see below are the proposed patches to fix these issues.
> 
> Thank you.

Thanks, queued for 2.7 and add Cc: qemu-stable@nongnu.org.

Paolo