Jacek Caban : ntdll: Check input buffer before server_read_file call and don' t touch event on error.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: a240bfcf9b614bc52036bf241177b1856ebb4a65
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=a240bfcf9b614bc52036bf2411...

Author: Jacek Caban <jacek(a)codeweavers.com>
Date:   Thu Jan 26 00:51:24 2017 +0100

ntdll: Check input buffer before server_read_file call and don't touch event on error.

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 dlls/ntdll/file.c       | 12 ++++--------
 dlls/ntdll/tests/file.c | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
index e1583da..93374c8 100644
--- a/dlls/ntdll/file.c
+++ b/dlls/ntdll/file.c
@@ -853,19 +853,15 @@ NTSTATUS WINAPI NtReadFile(HANDLE hFile, HANDLE hEvent,
 
     status = server_get_unix_fd( hFile, FILE_READ_DATA, &unix_handle,
                                  &needs_close, &type, &options );
+    if (status && status != STATUS_BAD_DEVICE_TYPE) return status;
+
+    if (!virtual_check_buffer_for_write( buffer, length )) return STATUS_ACCESS_VIOLATION;
+
     if (status == STATUS_BAD_DEVICE_TYPE)
         return server_read_file( hFile, hEvent, apc, apc_user, io_status, buffer, length, offset, key );
 
-    if (status) return status;
-
     async_read = !(options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT));
 
-    if (!virtual_check_buffer_for_write( buffer, length ))
-    {
-        status = STATUS_ACCESS_VIOLATION;
-        goto done;
-    }
-
     if (type == FD_TYPE_FILE)
     {
         if (async_read && (!offset || offset->QuadPart < 0))
diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
index 586b2c9..d56843d 100644
--- a/dlls/ntdll/tests/file.c
+++ b/dlls/ntdll/tests/file.c
@@ -3451,13 +3451,15 @@ static void test_read_write(void)
 {
     static const char contents[14] = "1234567890abcd";
     char buf[256];
-    HANDLE hfile;
+    HANDLE hfile, event;
     OVERLAPPED ovl;
     IO_STATUS_BLOCK iob;
     DWORD ret, bytes, status, off;
     LARGE_INTEGER offset;
     LONG i;
 
+    event = CreateEventA( NULL, TRUE, FALSE, NULL );
+
     U(iob).Status = -1;
     iob.Information = -1;
     offset.QuadPart = 0;
@@ -3469,6 +3471,14 @@ static void test_read_write(void)
     U(iob).Status = -1;
     iob.Information = -1;
     offset.QuadPart = 0;
+    status = pNtReadFile(INVALID_HANDLE_VALUE, 0, NULL, NULL, &iob, NULL, sizeof(buf), &offset, NULL);
+    ok(status == STATUS_OBJECT_TYPE_MISMATCH || status == STATUS_INVALID_HANDLE, "expected STATUS_OBJECT_TYPE_MISMATCH, got %#x\n", status);
+    ok(U(iob).Status == -1, "expected -1, got %#x\n", U(iob).Status);
+    ok(iob.Information == -1, "expected -1, got %lu\n", iob.Information);
+
+    U(iob).Status = -1;
+    iob.Information = -1;
+    offset.QuadPart = 0;
     status = pNtWriteFile(INVALID_HANDLE_VALUE, 0, NULL, NULL, &iob, buf, sizeof(buf), &offset, NULL);
     ok(status == STATUS_OBJECT_TYPE_MISMATCH || status == STATUS_INVALID_HANDLE, "expected STATUS_OBJECT_TYPE_MISMATCH, got %#x\n", status);
     ok(U(iob).Status == -1, "expected -1, got %#x\n", U(iob).Status);
@@ -3493,6 +3503,24 @@ static void test_read_write(void)
 
     U(iob).Status = -1;
     iob.Information = -1;
+    SetEvent(event);
+    status = pNtReadFile(hfile, event, NULL, NULL, &iob, NULL, sizeof(contents), NULL, NULL);
+    ok(status == STATUS_ACCESS_VIOLATION, "expected STATUS_ACCESS_VIOLATION, got %#x\n", status);
+    ok(U(iob).Status == -1, "expected -1, got %#x\n", U(iob).Status);
+    ok(iob.Information == -1, "expected -1, got %lu\n", iob.Information);
+    ok(is_signaled(event), "event is not signaled\n");
+
+    U(iob).Status = -1;
+    iob.Information = -1;
+    SetEvent(event);
+    status = pNtReadFile(hfile, event, NULL, NULL, &iob, (void*)0xdeadbeef, sizeof(contents), NULL, NULL);
+    ok(status == STATUS_ACCESS_VIOLATION, "expected STATUS_ACCESS_VIOLATION, got %#x\n", status);
+    ok(U(iob).Status == -1, "expected -1, got %#x\n", U(iob).Status);
+    ok(iob.Information == -1, "expected -1, got %lu\n", iob.Information);
+    ok(is_signaled(event), "event is not signaled\n");
+
+    U(iob).Status = -1;
+    iob.Information = -1;
     status = pNtWriteFile(hfile, 0, NULL, NULL, &iob, contents, 7, NULL, NULL);
     ok(status == STATUS_SUCCESS, "NtWriteFile error %#x\n", status);
     ok(U(iob).Status == STATUS_SUCCESS, "expected STATUS_SUCCESS, got %#x\n", U(iob).Status);
@@ -4156,6 +4184,7 @@ static void test_read_write(void)
     off = SetFilePointer(hfile, 0, NULL, FILE_CURRENT);
     ok(off == 0, "expected 0, got %u\n", off);
 
+    CloseHandle(event);
     CloseHandle(hfile);
 }