e2c46b
e2c46b
# HG changeset patch
e2c46b
# User Jed Davis <jld@mozilla.com>
e2c46b
# Date 1526943705 21600
e2c46b
# Node ID 6bb3adfa15c6877f7874429462dad88f8c978c4f
e2c46b
# Parent  4c71c8454879c841871ecf3afb7dbdc96bad97fc
e2c46b
Bug 1436242 - Avoid undefined behavior in IPC fd-passing code.  r=froydnj
e2c46b
e2c46b
MozReview-Commit-ID: 3szIPUssgF5
e2c46b
e2c46b
diff --git a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
e2c46b
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
e2c46b
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
e2c46b
@@ -418,20 +418,37 @@ bool Channel::ChannelImpl::ProcessIncomi
e2c46b
     const int* fds;
e2c46b
     unsigned num_fds;
e2c46b
     unsigned fds_i = 0;  // the index of the first unused descriptor
e2c46b
 
e2c46b
     if (input_overflow_fds_.empty()) {
e2c46b
       fds = wire_fds;
e2c46b
       num_fds = num_wire_fds;
e2c46b
     } else {
e2c46b
-      const size_t prev_size = input_overflow_fds_.size();
e2c46b
-      input_overflow_fds_.resize(prev_size + num_wire_fds);
e2c46b
-      memcpy(&input_overflow_fds_[prev_size], wire_fds,
e2c46b
-             num_wire_fds * sizeof(int));
e2c46b
+      // This code may look like a no-op in the case where
e2c46b
+      // num_wire_fds == 0, but in fact:
e2c46b
+      //
e2c46b
+      // 1. wire_fds will be nullptr, so passing it to memcpy is
e2c46b
+      // undefined behavior according to the C standard, even though
e2c46b
+      // the memcpy length is 0.
e2c46b
+      //
e2c46b
+      // 2. prev_size will be an out-of-bounds index for
e2c46b
+      // input_overflow_fds_; this is undefined behavior according to
e2c46b
+      // the C++ standard, even though the element only has its
e2c46b
+      // pointer taken and isn't accessed (and the corresponding
e2c46b
+      // operation on a C array would be defined).
e2c46b
+      //
e2c46b
+      // UBSan makes #1 a fatal error, and assertions in libstdc++ do
e2c46b
+      // the same for #2 if enabled.
e2c46b
+      if (num_wire_fds > 0) {
e2c46b
+        const size_t prev_size = input_overflow_fds_.size();
e2c46b
+        input_overflow_fds_.resize(prev_size + num_wire_fds);
e2c46b
+        memcpy(&input_overflow_fds_[prev_size], wire_fds,
e2c46b
+               num_wire_fds * sizeof(int));
e2c46b
+      }
e2c46b
       fds = &input_overflow_fds_[0];
e2c46b
       num_fds = input_overflow_fds_.size();
e2c46b
     }
e2c46b
 
e2c46b
     // The data for the message we're currently reading consists of any data
e2c46b
     // stored in incoming_message_ followed by data in input_buf_ (followed by
e2c46b
     // other messages).
e2c46b
 
e2c46b