[Backport] CVE-2020-15962: Insufficient policy enforcement in serial

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2393001:
serial: Check that port is open before reading or writing

This change adds checks to the platform-specific implementations
of Read() and Write() to make sure that the file descriptor is
valid before. This makes the assumptions validated by later DCHECK
correct.

This cannot be done in the platform-independent layer because test
code depends on being able to call some SerialIoHandler methods
without an actual file descriptor.

Bug: 1121836
Change-Id: If182404cf10a2f3b445b9c80b75fed5df6b5ab4b
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 20 insertions, 4 deletions

diff --git a/chromium/device/serial/serial_io_handler_posix.cc b/chromium/device/serial/serial_io_handler_posix.cc
index d6ed5e1f889..f87b5379d6e 100644
--- a/chromium/device/serial/serial_io_handler_posix.cc
+++ b/chromium/device/serial/serial_io_handler_posix.cc
@@ -121,7 +121,11 @@ scoped_refptr<SerialIoHandler> SerialIoHandler::Create(
 void SerialIoHandlerPosix::ReadImpl() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(pending_read_buffer());
-  DCHECK(file().IsValid());
+
+  if (file().IsValid()) {
+    QueueReadCompleted(0, mojom::SerialReceiveError::DISCONNECTED);
+    return;
+  }
 
   // Try to read immediately. This is needed because on some platforms
   // (e.g., OSX) there may not be a notification from the message loop
@@ -133,7 +137,11 @@ void SerialIoHandlerPosix::ReadImpl() {
 void SerialIoHandlerPosix::WriteImpl() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(pending_write_buffer());
-  DCHECK(file().IsValid());
+
+  if (file().IsValid()) {
+    QueueWriteCompleted(0, mojom::SerialSendError::DISCONNECTED);
+    return;
+  }
 
   EnsureWatchingWrites();
 }
diff --git a/chromium/device/serial/serial_io_handler_win.cc b/chromium/device/serial/serial_io_handler_win.cc
index 26efe2c9081..4b69bfae756 100644
--- a/chromium/device/serial/serial_io_handler_win.cc
+++ b/chromium/device/serial/serial_io_handler_win.cc
@@ -268,7 +268,11 @@ bool SerialIoHandlerWin::PostOpen() {
 void SerialIoHandlerWin::ReadImpl() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(pending_read_buffer());
-  DCHECK(file().IsValid());
+
+  if (!file().IsValid()) {
+    QueueReadCompleted(0, mojom::SerialReceiveError::DISCONNECTED);
+    return;
+  }
 
   if (!SetCommMask(file().GetPlatformFile(), EV_RXCHAR)) {
     VPLOG(1) << "Failed to set serial event flags";
@@ -287,7 +291,11 @@ void SerialIoHandlerWin::ReadImpl() {
 void SerialIoHandlerWin::WriteImpl() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(pending_write_buffer());
-  DCHECK(file().IsValid());
+
+  if (!file().IsValid()) {
+    QueueWriteCompleted(0, mojom::SerialSendError::DISCONNECTED);
+    return;
+  }
 
   BOOL ok = ::WriteFile(file().GetPlatformFile(),
                         pending_write_buffer(),