[ros-diffs] [reactos] 01/01: [BLUE] Add IOCTL buffers validation. Addendum to commit bfd8a848.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=afb14bff33efedffa796df...

commit afb14bff33efedffa796df27fa8d288b1ee1c504
Author:     Hermès Bélusca-Maïto [e-mail]
AuthorDate: Tue Dec 24 13:27:15 2019 +0100
Commit:     Hermès Bélusca-Maïto [e-mail]
CommitDate: Fri Jan 3 17:17:03 2020 +0100

[BLUE] Add IOCTL buffers validation. Addendum to commit bfd8a848.

Also convert all sizes and positions of CONSOLE_DRAW to USHORT since
    this is the standard type for all console buffer positions & sizes
    (minimum value 0, maximum value 0xFFFF == 65535).
---
 drivers/setup/blue/blue.c                   | 459 +++++++++++++++++++++-------
 sdk/include/reactos/drivers/blue/ntddblue.h |  20 +-
 2 files changed, 363 insertions(+), 116 deletions(-)

diff --git a/drivers/setup/blue/blue.c b/drivers/setup/blue/blue.c
index 2558506fb53..def5049fa32 100644
--- a/drivers/setup/blue/blue.c
+++ b/drivers/setup/blue/blue.c
@@ -292,7 +292,7 @@ ScrInbvCleanup(VOID)
     ThreadHandle = InterlockedExchangePointer((PVOID*)&InbvThreadHandle, NULL);
     if (ThreadHandle)
     {
-        KeWaitForSingleObject(&ThreadHandle, Executive, KernelMode, FALSE, NULL);
+        ZwWaitForSingleObject(ThreadHandle, Executive, KernelMode, FALSE, NULL);
         /* Close its handle */
         ObCloseHandle(ThreadHandle, KernelMode);
     }
@@ -658,14 +658,21 @@ ScrResetScreen(
     return TRUE; // STATUS_SUCCESS;
 }

-static DRIVER_DISPATCH ScrCreate;
+static DRIVER_DISPATCH ScrCreateClose;
 static NTSTATUS
 NTAPI
-ScrCreate(
+ScrCreateClose(
     _In_ PDEVICE_OBJECT DeviceObject,
     _In_ PIRP Irp)
 {
-    // Irp->IoStatus.Information = FILE_OPENED;
+    PIO_STACK_LOCATION stk = IoGetCurrentIrpStackLocation(Irp);
+
+    UNREFERENCED_PARAMETER(DeviceObject);
+
+    if (stk->MajorFunction == IRP_MJ_CREATE)
+        Irp->IoStatus.Information = FILE_OPENED;
+    // else: IRP_MJ_CLOSE
+
     Irp->IoStatus.Status = STATUS_SUCCESS;
     IoCompleteRequest(Irp, IO_NO_INCREMENT);
     return STATUS_SUCCESS;
@@ -824,7 +831,7 @@ ScrWrite(
     Status = STATUS_SUCCESS;

Irp->IoStatus.Status = Status;
-    IoCompleteRequest(Irp, IO_NO_INCREMENT);
+    IoCompleteRequest(Irp, IO_VIDEO_INCREMENT);

return Status;
 }
@@ -838,29 +845,48 @@ ScrIoControl(
 {
     NTSTATUS Status;
     PIO_STACK_LOCATION stk = IoGetCurrentIrpStackLocation(Irp);
-    PDEVICE_EXTENSION DeviceExtension;
+    PDEVICE_EXTENSION DeviceExtension = DeviceObject->DeviceExtension;

-    DeviceExtension = DeviceObject->DeviceExtension;
     switch (stk->Parameters.DeviceIoControl.IoControlCode)
     {
         case IOCTL_CONSOLE_RESET_SCREEN:
         {
-            BOOLEAN Enable = !!*(PULONG)Irp->AssociatedIrp.SystemBuffer;
+            BOOLEAN Enable;
+
+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(ULONG))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            Enable = !!*(PULONG)Irp->AssociatedIrp.SystemBuffer;

/* Fully enable or disable the screen */
             Status = (ScrResetScreen(DeviceExtension, TRUE, Enable)
                         ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL);
-
             Irp->IoStatus.Information = 0;
             break;
         }

case IOCTL_CONSOLE_GET_SCREEN_BUFFER_INFO:
         {
-            PCONSOLE_SCREEN_BUFFER_INFO pcsbi = (PCONSOLE_SCREEN_BUFFER_INFO)Irp->AssociatedIrp.SystemBuffer;
+            PCONSOLE_SCREEN_BUFFER_INFO pcsbi;
             USHORT rows = DeviceExtension->Rows;
             USHORT columns = DeviceExtension->Columns;

+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(CONSOLE_SCREEN_BUFFER_INFO))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            pcsbi = (PCONSOLE_SCREEN_BUFFER_INFO)Irp->AssociatedIrp.SystemBuffer;
+            RtlZeroMemory(pcsbi, sizeof(CONSOLE_SCREEN_BUFFER_INFO));
+
             pcsbi->dwSize.X = columns;
             pcsbi->dwSize.Y = rows;

@@ -884,7 +910,17 @@ ScrIoControl(

case IOCTL_CONSOLE_SET_SCREEN_BUFFER_INFO:
         {
-            PCONSOLE_SCREEN_BUFFER_INFO pcsbi = (PCONSOLE_SCREEN_BUFFER_INFO)Irp->AssociatedIrp.SystemBuffer;
+            PCONSOLE_SCREEN_BUFFER_INFO pcsbi;
+
+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(CONSOLE_SCREEN_BUFFER_INFO))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            pcsbi = (PCONSOLE_SCREEN_BUFFER_INFO)Irp->AssociatedIrp.SystemBuffer;

if ( pcsbi->dwCursorPosition.X < 0 || pcsbi->dwCursorPosition.X >= DeviceExtension->Columns ||
                  pcsbi->dwCursorPosition.Y < 0 || pcsbi->dwCursorPosition.Y >= DeviceExtension->Rows )
@@ -911,7 +947,18 @@ ScrIoControl(

case IOCTL_CONSOLE_GET_CURSOR_INFO:
         {
-            PCONSOLE_CURSOR_INFO pcci = (PCONSOLE_CURSOR_INFO)Irp->AssociatedIrp.SystemBuffer;
+            PCONSOLE_CURSOR_INFO pcci;
+
+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(CONSOLE_CURSOR_INFO))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            pcci = (PCONSOLE_CURSOR_INFO)Irp->AssociatedIrp.SystemBuffer;
+            RtlZeroMemory(pcci, sizeof(CONSOLE_CURSOR_INFO));

pcci->dwSize = DeviceExtension->CursorSize;
             pcci->bVisible = DeviceExtension->CursorVisible;
@@ -923,7 +970,17 @@ ScrIoControl(

case IOCTL_CONSOLE_SET_CURSOR_INFO:
         {
-            PCONSOLE_CURSOR_INFO pcci = (PCONSOLE_CURSOR_INFO)Irp->AssociatedIrp.SystemBuffer;
+            PCONSOLE_CURSOR_INFO pcci;
+
+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(CONSOLE_CURSOR_INFO))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            pcci = (PCONSOLE_CURSOR_INFO)Irp->AssociatedIrp.SystemBuffer;

DeviceExtension->CursorSize = pcci->dwSize;
             DeviceExtension->CursorVisible = pcci->bVisible;
@@ -937,7 +994,18 @@ ScrIoControl(

case IOCTL_CONSOLE_GET_MODE:
         {
-            PCONSOLE_MODE pcm = (PCONSOLE_MODE)Irp->AssociatedIrp.SystemBuffer;
+            PCONSOLE_MODE pcm;
+
+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(CONSOLE_MODE))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            pcm = (PCONSOLE_MODE)Irp->AssociatedIrp.SystemBuffer;
+            RtlZeroMemory(pcm, sizeof(CONSOLE_MODE));

pcm->dwMode = DeviceExtension->Mode;

@@ -948,8 +1016,17 @@ ScrIoControl(

case IOCTL_CONSOLE_SET_MODE:
         {
-            PCONSOLE_MODE pcm = (PCONSOLE_MODE)Irp->AssociatedIrp.SystemBuffer;
+            PCONSOLE_MODE pcm;

+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(CONSOLE_MODE))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            pcm = (PCONSOLE_MODE)Irp->AssociatedIrp.SystemBuffer;
             DeviceExtension->Mode = pcm->dwMode;

Irp->IoStatus.Information = 0;
@@ -959,17 +1036,31 @@ ScrIoControl(

case IOCTL_CONSOLE_FILL_OUTPUT_ATTRIBUTE:
         {
-            POUTPUT_ATTRIBUTE Buf = (POUTPUT_ATTRIBUTE)Irp->AssociatedIrp.SystemBuffer;
+            POUTPUT_ATTRIBUTE Buf;
             PUCHAR vidmem;
             ULONG offset;
             ULONG dwCount;
-            ULONG nMaxLength = Buf->nLength;
+            ULONG nMaxLength;
+
+            /* Validate input and output buffers */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength  < sizeof(OUTPUT_ATTRIBUTE) ||
+                stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(OUTPUT_ATTRIBUTE))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            Buf = (POUTPUT_ATTRIBUTE)Irp->AssociatedIrp.SystemBuffer;
+            nMaxLength = Buf->nLength;
+
+            Buf->dwTransfered = 0;
+            Irp->IoStatus.Information = sizeof(OUTPUT_ATTRIBUTE);

if ( Buf->dwCoord.X < 0 || Buf->dwCoord.X >= DeviceExtension->Columns ||
-                 Buf->dwCoord.Y < 0 || Buf->dwCoord.Y >= DeviceExtension->Rows )
+                 Buf->dwCoord.Y < 0 || Buf->dwCoord.Y >= DeviceExtension->Rows    ||
+                 nMaxLength == 0 )
             {
-                Buf->dwTransfered = 0;
-                Irp->IoStatus.Information = 0;
                 Status = STATUS_SUCCESS;
                 break;
             }
@@ -987,39 +1078,63 @@ ScrIoControl(
                 {
                     vidmem[offset + (dwCount * 2)] = (char)Buf->wAttribute;
                 }
+                Buf->dwTransfered = dwCount;
             }

-            Buf->dwTransfered = nMaxLength;
-
-            Irp->IoStatus.Information = 0;
             Status = STATUS_SUCCESS;
             break;
         }

case IOCTL_CONSOLE_READ_OUTPUT_ATTRIBUTE:
         {
-            POUTPUT_ATTRIBUTE Buf = (POUTPUT_ATTRIBUTE)Irp->AssociatedIrp.SystemBuffer;
-            PUSHORT pAttr = (PUSHORT)MmGetSystemAddressForMdl(Irp->MdlAddress);
+            POUTPUT_ATTRIBUTE Buf;
+            PUSHORT pAttr;
             PUCHAR vidmem;
             ULONG offset;
             ULONG dwCount;
             ULONG nMaxLength;

+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(OUTPUT_ATTRIBUTE))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            Buf = (POUTPUT_ATTRIBUTE)Irp->AssociatedIrp.SystemBuffer;
+            Irp->IoStatus.Information = 0;
+
+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength == 0)
+            {
+                Status = STATUS_SUCCESS;
+                break;
+            }
+            ASSERT(Irp->MdlAddress);
+            pAttr = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
+            if (pAttr == NULL)
+            {
+                Status = STATUS_INSUFFICIENT_RESOURCES;
+                break;
+            }
+
             if ( Buf->dwCoord.X < 0 || Buf->dwCoord.X >= DeviceExtension->Columns ||
                  Buf->dwCoord.Y < 0 || Buf->dwCoord.Y >= DeviceExtension->Rows )
             {
-                Buf->dwTransfered = 0;
-                Irp->IoStatus.Information = 0;
                 Status = STATUS_SUCCESS;
                 break;
             }

+            nMaxLength = stk->Parameters.DeviceIoControl.OutputBufferLength;
+            nMaxLength /= sizeof(USHORT);
+
             if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
             {
                 vidmem = DeviceExtension->VideoMemory;
                 offset = (Buf->dwCoord.X + Buf->dwCoord.Y * DeviceExtension->Columns) * 2 + 1;

-                nMaxLength = min(stk->Parameters.DeviceIoControl.OutputBufferLength,
+                nMaxLength = min(nMaxLength,
                                  (DeviceExtension->Rows - Buf->dwCoord.Y)
                                     * DeviceExtension->Columns - Buf->dwCoord.X);

@@ -1027,75 +1142,123 @@ ScrIoControl(
                 {
                     *((PCHAR)pAttr) = vidmem[offset + (dwCount * 2)];
                 }
-
-                Buf->dwTransfered = dwCount;
-            }
-            else
-            {
-                Buf->dwTransfered = 0;
+                Irp->IoStatus.Information = dwCount * sizeof(USHORT);
             }

-            Irp->IoStatus.Information = sizeof(OUTPUT_ATTRIBUTE);
             Status = STATUS_SUCCESS;
             break;
         }

case IOCTL_CONSOLE_WRITE_OUTPUT_ATTRIBUTE:
         {
-            PCOORD pCoord = (PCOORD)MmGetSystemAddressForMdl(Irp->MdlAddress);
-            PCHAR pAttr = (PCHAR)(pCoord + 1);
+            COORD dwCoord;
+            PCOORD pCoord;
+            PUSHORT pAttr;
             PUCHAR vidmem;
             ULONG offset;
             ULONG dwCount;
             ULONG nMaxLength;

-            if ( pCoord->X < 0 || pCoord->X >= DeviceExtension->Columns ||
-                 pCoord->Y < 0 || pCoord->Y >= DeviceExtension->Rows )
+            //
+            // NOTE: For whatever reason no OUTPUT_ATTRIBUTE structure
+            // is used for this IOCTL.
+            //
+
+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(COORD))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->MdlAddress);
+            pCoord = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
+            if (pCoord == NULL)
+            {
+                Status = STATUS_INSUFFICIENT_RESOURCES;
+                break;
+            }
+            /* Capture the input info data */
+            dwCoord = *pCoord;
+
+            nMaxLength = stk->Parameters.DeviceIoControl.OutputBufferLength - sizeof(COORD);
+            nMaxLength /= sizeof(USHORT);
+
+            Irp->IoStatus.Information = 0;
+
+            if ( dwCoord.X < 0 || dwCoord.X >= DeviceExtension->Columns ||
+                 dwCoord.Y < 0 || dwCoord.Y >= DeviceExtension->Rows    ||
+                 nMaxLength == 0 )
             {
-                Irp->IoStatus.Information = 0;
                 Status = STATUS_SUCCESS;
                 break;
             }

+            pAttr = (PUSHORT)(pCoord + 1);
+
             if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
             {
                 vidmem = DeviceExtension->VideoMemory;
-                offset = (pCoord->X + pCoord->Y * DeviceExtension->Columns) * 2 + 1;
+                offset = (dwCoord.X + dwCoord.Y * DeviceExtension->Columns) * 2 + 1;

-                nMaxLength = min(stk->Parameters.DeviceIoControl.OutputBufferLength - sizeof(COORD),
-                                 (DeviceExtension->Rows - pCoord->Y)
-                                    * DeviceExtension->Columns - pCoord->X);
+                nMaxLength = min(nMaxLength,
+                                 (DeviceExtension->Rows - dwCoord.Y)
+                                    * DeviceExtension->Columns - dwCoord.X);

for (dwCount = 0; dwCount < nMaxLength; dwCount++, pAttr++)
                 {
-                    vidmem[offset + (dwCount * 2)] = *pAttr;
+                    vidmem[offset + (dwCount * 2)] = *((PCHAR)pAttr);
                 }
+                Irp->IoStatus.Information = dwCount * sizeof(USHORT);
             }

-            Irp->IoStatus.Information = 0;
             Status = STATUS_SUCCESS;
             break;
         }

case IOCTL_CONSOLE_SET_TEXT_ATTRIBUTE:
-            DeviceExtension->CharAttribute = (USHORT)*(PUSHORT)Irp->AssociatedIrp.SystemBuffer;
+        {
+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(USHORT))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            DeviceExtension->CharAttribute = *(PUSHORT)Irp->AssociatedIrp.SystemBuffer;
+
             Irp->IoStatus.Information = 0;
             Status = STATUS_SUCCESS;
             break;
+        }

case IOCTL_CONSOLE_FILL_OUTPUT_CHARACTER:
         {
-            POUTPUT_CHARACTER Buf = (POUTPUT_CHARACTER)Irp->AssociatedIrp.SystemBuffer;
+            POUTPUT_CHARACTER Buf;
             PUCHAR vidmem;
             ULONG offset;
             ULONG dwCount;
-            ULONG nMaxLength = Buf->nLength;
+            ULONG nMaxLength;
+
+            /* Validate input and output buffers */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength  < sizeof(OUTPUT_CHARACTER) ||
+                stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(OUTPUT_CHARACTER))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            Buf = (POUTPUT_CHARACTER)Irp->AssociatedIrp.SystemBuffer;
+            nMaxLength = Buf->nLength;
+
+            Buf->dwTransfered = 0;
+            Irp->IoStatus.Information = sizeof(OUTPUT_CHARACTER);

if ( Buf->dwCoord.X < 0 || Buf->dwCoord.X >= DeviceExtension->Columns ||
-                 Buf->dwCoord.Y < 0 || Buf->dwCoord.Y >= DeviceExtension->Rows )
+                 Buf->dwCoord.Y < 0 || Buf->dwCoord.Y >= DeviceExtension->Rows    ||
+                 nMaxLength == 0 )
             {
-                Buf->dwTransfered = 0;
-                Irp->IoStatus.Information = 0;
                 Status = STATUS_SUCCESS;
                 break;
             }
@@ -1113,39 +1276,62 @@ ScrIoControl(
                 {
                     vidmem[offset + (dwCount * 2)] = (char)Buf->cCharacter;
                 }
+                Buf->dwTransfered = dwCount;
             }

-            Buf->dwTransfered = nMaxLength;
-
-            Irp->IoStatus.Information = 0;
             Status = STATUS_SUCCESS;
             break;
         }

case IOCTL_CONSOLE_READ_OUTPUT_CHARACTER:
         {
-            POUTPUT_CHARACTER Buf = (POUTPUT_CHARACTER)Irp->AssociatedIrp.SystemBuffer;
-            PCHAR pChar = (PCHAR)MmGetSystemAddressForMdl(Irp->MdlAddress);
+            POUTPUT_CHARACTER Buf;
+            PCHAR pChar;
             PUCHAR vidmem;
             ULONG offset;
             ULONG dwCount;
             ULONG nMaxLength;

+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(OUTPUT_CHARACTER))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            Buf = (POUTPUT_CHARACTER)Irp->AssociatedIrp.SystemBuffer;
+            Irp->IoStatus.Information = 0;
+
+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength == 0)
+            {
+                Status = STATUS_SUCCESS;
+                break;
+            }
+            ASSERT(Irp->MdlAddress);
+            pChar = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
+            if (pChar == NULL)
+            {
+                Status = STATUS_INSUFFICIENT_RESOURCES;
+                break;
+            }
+
             if ( Buf->dwCoord.X < 0 || Buf->dwCoord.X >= DeviceExtension->Columns ||
                  Buf->dwCoord.Y < 0 || Buf->dwCoord.Y >= DeviceExtension->Rows )
             {
-                Buf->dwTransfered = 0;
-                Irp->IoStatus.Information = 0;
                 Status = STATUS_SUCCESS;
                 break;
             }

+            nMaxLength = stk->Parameters.DeviceIoControl.OutputBufferLength;
+
             if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
             {
                 vidmem = DeviceExtension->VideoMemory;
                 offset = (Buf->dwCoord.X + Buf->dwCoord.Y * DeviceExtension->Columns) * 2;

-                nMaxLength = min(stk->Parameters.DeviceIoControl.OutputBufferLength,
+                nMaxLength = min(nMaxLength,
                                  (DeviceExtension->Rows - Buf->dwCoord.Y)
                                     * DeviceExtension->Columns - Buf->dwCoord.X);

@@ -1153,72 +1339,135 @@ ScrIoControl(
                 {
                     *pChar = vidmem[offset + (dwCount * 2)];
                 }
-
-                Buf->dwTransfered = dwCount;
-            }
-            else
-            {
-                Buf->dwTransfered = 0;
+                Irp->IoStatus.Information = dwCount * sizeof(CHAR);
             }

-            Irp->IoStatus.Information = sizeof(OUTPUT_ATTRIBUTE);
             Status = STATUS_SUCCESS;
             break;
         }

case IOCTL_CONSOLE_WRITE_OUTPUT_CHARACTER:
         {
-            PCOORD pCoord = (PCOORD)MmGetSystemAddressForMdl(Irp->MdlAddress);
-            PCHAR pChar = (PCHAR)(pCoord + 1);
+            COORD dwCoord;
+            PCOORD pCoord;
+            PCHAR pChar;
             PUCHAR vidmem;
             ULONG offset;
             ULONG dwCount;
             ULONG nMaxLength;

-            if ( pCoord->X < 0 || pCoord->X >= DeviceExtension->Columns ||
-                 pCoord->Y < 0 || pCoord->Y >= DeviceExtension->Rows )
+            //
+            // NOTE: For whatever reason no OUTPUT_CHARACTER structure
+            // is used for this IOCTL.
+            //
+
+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(COORD))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->MdlAddress);
+            pCoord = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
+            if (pCoord == NULL)
+            {
+                Status = STATUS_INSUFFICIENT_RESOURCES;
+                break;
+            }
+            /* Capture the input info data */
+            dwCoord = *pCoord;
+
+            nMaxLength = stk->Parameters.DeviceIoControl.OutputBufferLength - sizeof(COORD);
+            Irp->IoStatus.Information = 0;
+
+            if ( dwCoord.X < 0 || dwCoord.X >= DeviceExtension->Columns ||
+                 dwCoord.Y < 0 || dwCoord.Y >= DeviceExtension->Rows    ||
+                 nMaxLength == 0 )
             {
-                Irp->IoStatus.Information = 0;
                 Status = STATUS_SUCCESS;
                 break;
             }

+            pChar = (PCHAR)(pCoord + 1);
+
             if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
             {
                 vidmem = DeviceExtension->VideoMemory;
-                offset = (pCoord->X + pCoord->Y * DeviceExtension->Columns) * 2;
+                offset = (dwCoord.X + dwCoord.Y * DeviceExtension->Columns) * 2;

-                nMaxLength = min(stk->Parameters.DeviceIoControl.OutputBufferLength - sizeof(COORD),
-                                 (DeviceExtension->Rows - pCoord->Y)
-                                    * DeviceExtension->Columns - pCoord->X);
+                nMaxLength = min(nMaxLength,
+                                 (DeviceExtension->Rows - dwCoord.Y)
+                                    * DeviceExtension->Columns - dwCoord.X);

for (dwCount = 0; dwCount < nMaxLength; dwCount++, pChar++)
                 {
                     vidmem[offset + (dwCount * 2)] = *pChar;
                 }
+                Irp->IoStatus.Information = dwCount * sizeof(CHAR);
             }

-            Irp->IoStatus.Information = 0;
             Status = STATUS_SUCCESS;
             break;
         }

case IOCTL_CONSOLE_DRAW:
         {
-            PCONSOLE_DRAW ConsoleDraw;
+            CONSOLE_DRAW ConsoleDraw;
+            PCONSOLE_DRAW pConsoleDraw;
             PUCHAR Src, Dest;
             UINT32 SrcDelta, DestDelta, i;

+            /* Validate output buffer */
+            if (stk->Parameters.DeviceIoControl.OutputBufferLength < sizeof(CONSOLE_DRAW))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+            ASSERT(Irp->MdlAddress);
+            pConsoleDraw = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority);
+            if (pConsoleDraw == NULL)
+            {
+                Status = STATUS_INSUFFICIENT_RESOURCES;
+                break;
+            }
+            /* Capture the input info data */
+            ConsoleDraw = *pConsoleDraw;
+
+            /* Check whether we have the size for the header plus the data area */
+            if ((stk->Parameters.DeviceIoControl.OutputBufferLength - sizeof(CONSOLE_DRAW)) / 2
+                    < ((ULONG)ConsoleDraw.SizeX * (ULONG)ConsoleDraw.SizeY))
+            {
+                Status = STATUS_INVALID_BUFFER_SIZE;
+                break;
+            }
+
+            // TODO: For the moment if the ConsoleDraw rectangle has borders
+            // out of the screen-buffer we just bail out. Would it be better
+            // to actually clip the rectangle within its borders instead?
+            if ( ConsoleDraw.X < 0 || ConsoleDraw.X >= DeviceExtension->Columns ||
+                 ConsoleDraw.Y < 0 || ConsoleDraw.Y >= DeviceExtension->Rows )
+            {
+                Status = STATUS_SUCCESS;
+                break;
+            }
+            if ( ConsoleDraw.SizeX >= DeviceExtension->Columns - ConsoleDraw.X ||
+                 ConsoleDraw.SizeY >= DeviceExtension->Rows    - ConsoleDraw.Y )
+            {
+                Status = STATUS_SUCCESS;
+                break;
+            }
+
             if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
             {
-                ConsoleDraw = (PCONSOLE_DRAW) MmGetSystemAddressForMdl(Irp->MdlAddress);
-                Src = (PUCHAR) (ConsoleDraw + 1);
-                SrcDelta = ConsoleDraw->SizeX * 2;
+                Src = (PUCHAR)(pConsoleDraw + 1);
+                SrcDelta = ConsoleDraw.SizeX * 2;
                 Dest = DeviceExtension->VideoMemory +
-                        (ConsoleDraw->X + ConsoleDraw->Y * DeviceExtension->Columns) * 2;
+                        (ConsoleDraw.X + ConsoleDraw.Y * DeviceExtension->Columns) * 2;
                 DestDelta = DeviceExtension->Columns * 2;
+                /* 2 == sizeof(CHAR) + sizeof(BYTE) */

-                for (i = 0; i < ConsoleDraw->SizeY; i++)
+                /* Copy each line */
+                for (i = 0; i < ConsoleDraw.SizeY; i++)
                 {
                     RtlCopyMemory(Dest, Src, SrcDelta);
                     Src += SrcDelta;
@@ -1227,8 +1476,8 @@ ScrIoControl(
             }

/* Set the cursor position, clipping it to the screen */
-            DeviceExtension->CursorX = min(max(ConsoleDraw->CursorX, 0), DeviceExtension->Columns - 1);
-            DeviceExtension->CursorY = min(max(ConsoleDraw->CursorY, 0), DeviceExtension->Rows - 1);
+            DeviceExtension->CursorX = min(max(ConsoleDraw.CursorX, 0), DeviceExtension->Columns - 1);
+            DeviceExtension->CursorY = min(max(ConsoleDraw.CursorY, 0), DeviceExtension->Rows    - 1);
             if (DeviceExtension->Enabled)
                 ScrSetCursor(DeviceExtension);

@@ -1239,14 +1488,19 @@ ScrIoControl(

case IOCTL_CONSOLE_LOADFONT:
         {
-            ULONG CodePage = *(PULONG)Irp->AssociatedIrp.SystemBuffer;
-            DeviceExtension->CodePage = CodePage;
-
-            if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
+            /* Validate input buffer */
+            if (stk->Parameters.DeviceIoControl.InputBufferLength < sizeof(ULONG))
             {
-                /* Upload a font for the codepage if needed */
-                ScrLoadFontTable(CodePage);
+                Status = STATUS_INVALID_PARAMETER;
+                break;
             }
+            ASSERT(Irp->AssociatedIrp.SystemBuffer);
+
+            DeviceExtension->CodePage = *(PULONG)Irp->AssociatedIrp.SystemBuffer;
+
+            /* Upload a font for the codepage if needed */
+            if (DeviceExtension->Enabled && DeviceExtension->VideoMemory)
+                ScrLoadFontTable(DeviceExtension->CodePage);

Irp->IoStatus.Information = 0;
             Status = STATUS_SUCCESS;
@@ -1258,7 +1512,7 @@ ScrIoControl(
     }

Irp->IoStatus.Status = Status;
-    IoCompleteRequest(Irp, IO_NO_INCREMENT);
+    IoCompleteRequest(Irp, IO_VIDEO_INCREMENT);

return Status;
 }
@@ -1270,23 +1524,16 @@ ScrDispatch(
     _In_ PDEVICE_OBJECT DeviceObject,
     _In_ PIRP Irp)
 {
-    NTSTATUS Status;
     PIO_STACK_LOCATION stk = IoGetCurrentIrpStackLocation(Irp);

-    switch (stk->MajorFunction)
-    {
-        case IRP_MJ_CLOSE:
-            Status = STATUS_SUCCESS;
-            break;
+    UNREFERENCED_PARAMETER(DeviceObject);

-        default:
-            Status = STATUS_NOT_IMPLEMENTED;
-            break;
-    }
+    DPRINT1("ScrDispatch(0x%p): stk->MajorFunction = %lu UNIMPLEMENTED\n",
+            DeviceObject, stk->MajorFunction);

-    Irp->IoStatus.Status = Status;
+    Irp->IoStatus.Status = STATUS_NOT_IMPLEMENTED;
     IoCompleteRequest(Irp, IO_NO_INCREMENT);
-    return Status;
+    return STATUS_NOT_IMPLEMENTED;
 }

/*
@@ -1305,8 +1552,8 @@ DriverEntry(

DPRINT("Screen Driver 0.0.6\n");

-    DriverObject->MajorFunction[IRP_MJ_CREATE] = ScrCreate;
-    DriverObject->MajorFunction[IRP_MJ_CLOSE]  = ScrDispatch;
+    DriverObject->MajorFunction[IRP_MJ_CREATE] = ScrCreateClose;
+    DriverObject->MajorFunction[IRP_MJ_CLOSE]  = ScrCreateClose;
     DriverObject->MajorFunction[IRP_MJ_READ]   = ScrDispatch;
     DriverObject->MajorFunction[IRP_MJ_WRITE]  = ScrWrite;
     DriverObject->MajorFunction[IRP_MJ_DEVICE_CONTROL] = ScrIoControl;
diff --git a/sdk/include/reactos/drivers/blue/ntddblue.h b/sdk/include/reactos/drivers/blue/ntddblue.h
index 5c6df7bfff7..0d41d0f158d 100644
--- a/sdk/include/reactos/drivers/blue/ntddblue.h
+++ b/sdk/include/reactos/drivers/blue/ntddblue.h
@@ -11,13 +11,13 @@
 #define IOCTL_CONSOLE_SET_MODE                  CTL_CODE(FILE_DEVICE_SCREEN, 0x806, METHOD_BUFFERED, FILE_WRITE_ACCESS)

#define IOCTL_CONSOLE_FILL_OUTPUT_ATTRIBUTE     CTL_CODE(FILE_DEVICE_SCREEN, 0x810, METHOD_BUFFERED, FILE_WRITE_ACCESS)
-#define IOCTL_CONSOLE_READ_OUTPUT_ATTRIBUTE     CTL_CODE(FILE_DEVICE_SCREEN, 0x811, METHOD_OUT_DIRECT, FILE_ANY_ACCESS)
-#define IOCTL_CONSOLE_WRITE_OUTPUT_ATTRIBUTE    CTL_CODE(FILE_DEVICE_SCREEN, 0x812, METHOD_IN_DIRECT, FILE_ANY_ACCESS)
+#define IOCTL_CONSOLE_READ_OUTPUT_ATTRIBUTE     CTL_CODE(FILE_DEVICE_SCREEN, 0x811, METHOD_OUT_DIRECT, FILE_READ_ACCESS)
+#define IOCTL_CONSOLE_WRITE_OUTPUT_ATTRIBUTE    CTL_CODE(FILE_DEVICE_SCREEN, 0x812, METHOD_IN_DIRECT, FILE_WRITE_ACCESS)
 #define IOCTL_CONSOLE_SET_TEXT_ATTRIBUTE        CTL_CODE(FILE_DEVICE_SCREEN, 0x813, METHOD_BUFFERED, FILE_WRITE_ACCESS)

#define IOCTL_CONSOLE_FILL_OUTPUT_CHARACTER     CTL_CODE(FILE_DEVICE_SCREEN, 0x820, METHOD_BUFFERED, FILE_WRITE_ACCESS)
-#define IOCTL_CONSOLE_READ_OUTPUT_CHARACTER     CTL_CODE(FILE_DEVICE_SCREEN, 0x821, METHOD_OUT_DIRECT, FILE_ANY_ACCESS)
-#define IOCTL_CONSOLE_WRITE_OUTPUT_CHARACTER    CTL_CODE(FILE_DEVICE_SCREEN, 0x822, METHOD_IN_DIRECT, FILE_ANY_ACCESS)
+#define IOCTL_CONSOLE_READ_OUTPUT_CHARACTER     CTL_CODE(FILE_DEVICE_SCREEN, 0x821, METHOD_OUT_DIRECT, FILE_READ_ACCESS)
+#define IOCTL_CONSOLE_WRITE_OUTPUT_CHARACTER    CTL_CODE(FILE_DEVICE_SCREEN, 0x822, METHOD_IN_DIRECT, FILE_WRITE_ACCESS)

#define IOCTL_CONSOLE_DRAW                      CTL_CODE(FILE_DEVICE_SCREEN, 0x830, METHOD_IN_DIRECT, FILE_WRITE_ACCESS)

@@ -48,12 +48,12 @@ typedef struct tagOUTPUT_CHARACTER

typedef struct tagCONSOLE_DRAW
 {
-    ULONG X;       /* Origin */
-    ULONG Y;
-    ULONG SizeX;   /* Size of the screen buffer (chars) */
-    ULONG SizeY;
-    ULONG CursorX; /* New cursor position (screen-relative) */
-    ULONG CursorY;
+    USHORT X;       /* Origin */
+    USHORT Y;
+    USHORT SizeX;   /* Size of the screen buffer (chars) */
+    USHORT SizeY;
+    USHORT CursorX; /* New cursor position (screen-relative) */
+    USHORT CursorY;
     /* Followed by screen buffer in char/attrib format */
 } CONSOLE_DRAW, *PCONSOLE_DRAW;