github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick fix from chromium issue 1076703 (#24563)

Browse Source
This commit is contained in:
Cheng Zhao
2020-07-16 18:36:57 +09:00
committed by GitHub
parent df1e7a0816
commit f79b489baa
3 changed files with 204 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
patches/config.json
View File

@@ -7,5 +7,7 @@
"src/electron/patches/node": "src/third_party/electron_node",
"src/electron/patches/pdfium": "src/third_party/pdfium"
"src/electron/patches/pdfium": "src/third_party/pdfium",
"src/electron/patches/webrtc": "src/third_party/webrtc"
}

1
patches/webrtc/.patches Normal file
View File

@@ -0,0 +1 @@
backport_1076703.patch

200
patches/webrtc/backport_1076703.patch Normal file
View File

@@ -0,0 +1,200 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Thu, 4 Oct 2018 14:57:02 -0700
Subject: fix: prevent pointer from being sent in the clear over SCTP
[1076703] [High] [CVE-2020-6514]: Security: WebRTC: usrsctp is called with pointer as network address
Backport https://webrtc.googlesource.com/src/+/963cc1ef1336b52ca27742beb28bfbc211ed54d0
diff --git a/media/sctp/sctp_transport.cc b/media/sctp/sctp_transport.cc
index 40061a604870b0e7033449d114e3a9b8fa32d028..1a6dc334e1fd1f69a094e763d13f71f4c5292a5e 100644
--- a/media/sctp/sctp_transport.cc
+++ b/media/sctp/sctp_transport.cc
@@ -22,6 +22,7 @@ enum PreservedErrno {
#include <stdio.h>
#include <memory>
+#include <unordered_map>
#include "absl/algorithm/container.h"
#include "absl/base/attributes.h"
@@ -39,6 +40,7 @@ enum PreservedErrno {
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/string_utils.h"
+#include "rtc_base/thread_annotations.h"
#include "rtc_base/thread_checker.h"
#include "rtc_base/trace_event.h"
#include "usrsctplib/usrsctp.h"
@@ -72,6 +74,59 @@ enum PayloadProtocolIdentifier {
PPID_TEXT_LAST = 51
};
+// Maps SCTP transport ID to SctpTransport object, necessary in send threshold
+// callback and outgoing packet callback.
+// TODO(crbug.com/1076703): Remove once the underlying problem is fixed or
+// workaround is provided in usrsctp.
+class SctpTransportMap {
+ public:
+ SctpTransportMap() = default;
+
+ // Assigns a new unused ID to the following transport.
+ uintptr_t Register(cricket::SctpTransport* transport) {
+ rtc::CritScope cs(&lock_);
+ // usrsctp_connect fails with a value of 0...
+ if (next_id_ == 0) {
+ ++next_id_;
+ }
+ // In case we've wrapped around and need to find an empty spot from a
+ // removed transport. Assumes we'll never be full.
+ while (map_.find(next_id_) != map_.end()) {
+ ++next_id_;
+ if (next_id_ == 0) {
+ ++next_id_;
+ }
+ };
+ map_[next_id_] = transport;
+ return next_id_++;
+ }
+
+ // Returns true if found.
+ bool Deregister(uintptr_t id) {
+ rtc::CritScope cs(&lock_);
+ return map_.erase(id) > 0;
+ }
+
+ cricket::SctpTransport* Retrieve(uintptr_t id) const {
+ rtc::CritScope cs(&lock_);
+ auto it = map_.find(id);
+ if (it == map_.end()) {
+ return nullptr;
+ }
+ return it->second;
+ }
+
+ private:
+ rtc::CriticalSection lock_;
+
+ uintptr_t next_id_ RTC_GUARDED_BY(lock_) = 0;
+ std::unordered_map<uintptr_t, cricket::SctpTransport*> map_
+ RTC_GUARDED_BY(lock_);
+};
+
+// Should only be modified by UsrSctpWrapper.
+ABSL_CONST_INIT SctpTransportMap* g_transport_map_ = nullptr;
+
// Helper for logging SCTP messages.
#if defined(__GNUC__)
__attribute__((__format__(__printf__, 1, 2)))
@@ -242,9 +297,12 @@ class SctpTransport::UsrSctpWrapper {
// Set the number of default outgoing streams. This is the number we'll
// send in the SCTP INIT message.
usrsctp_sysctl_set_sctp_nr_outgoing_streams_default(kMaxSctpStreams);
+
+ g_transport_map_ = new SctpTransportMap();
}
static void UninitializeUsrSctp() {
+ delete g_transport_map_;
RTC_LOG(LS_INFO) << __FUNCTION__;
// usrsctp_finish() may fail if it's called too soon after the transports
// are
@@ -282,7 +340,14 @@ class SctpTransport::UsrSctpWrapper {
size_t length,
uint8_t tos,
uint8_t set_df) {
- SctpTransport* transport = static_cast<SctpTransport*>(addr);
+ SctpTransport* transport =
+ g_transport_map_->Retrieve(reinterpret_cast<uintptr_t>(addr));
+ if (!transport) {
+ RTC_LOG(LS_ERROR)
+ << "OnSctpOutboundPacket: Failed to get transport for socket ID "
+ << addr;
+ return EINVAL;
+ }
RTC_LOG(LS_VERBOSE) << "global OnSctpOutboundPacket():"
"addr: "
<< addr << "; length: " << length
@@ -392,14 +457,14 @@ class SctpTransport::UsrSctpWrapper {
return nullptr;
}
// usrsctp_getladdrs() returns the addresses bound to this socket, which
- // contains the SctpTransport* as sconn_addr. Read the pointer,
+ // contains the SctpTransport id as sconn_addr. Read the id,
// then free the list of addresses once we have the pointer. We only open
// AF_CONN sockets, and they should all have the sconn_addr set to the
- // pointer that created them, so [0] is as good as any other.
+ // id of the transport that created them, so [0] is as good as any other.
struct sockaddr_conn* sconn =
reinterpret_cast<struct sockaddr_conn*>(&addrs[0]);
- SctpTransport* transport =
- reinterpret_cast<SctpTransport*>(sconn->sconn_addr);
+ SctpTransport* transport = g_transport_map_->Retrieve(
+ reinterpret_cast<uintptr_t>(sconn->sconn_addr));
usrsctp_freeladdrs(addrs);
return transport;
@@ -779,9 +844,10 @@ bool SctpTransport::OpenSctpSocket() {
UsrSctpWrapper::DecrementUsrSctpUsageCount();
return false;
}
- // Register this class as an address for usrsctp. This is used by SCTP to
+ id_ = g_transport_map_->Register(this);
+ // Register our id as an address for usrsctp. This is used by SCTP to
// direct the packets received (by the created socket) to this class.
- usrsctp_register_address(this);
+ usrsctp_register_address(reinterpret_cast<void*>(id_));
return true;
}
@@ -872,7 +938,8 @@ void SctpTransport::CloseSctpSocket() {
// discarded instead of being sent.
usrsctp_close(sock_);
sock_ = nullptr;
- usrsctp_deregister_address(this);
+ usrsctp_deregister_address(reinterpret_cast<void*>(id_));
+ RTC_CHECK(g_transport_map_->Deregister(id_));
UsrSctpWrapper::DecrementUsrSctpUsageCount();
ready_to_send_data_ = false;
}
@@ -1003,7 +1070,7 @@ void SctpTransport::OnPacketRead(rtc::PacketTransportInternal* transport,
// will be will be given to the global OnSctpInboundData, and then,
// marshalled by the AsyncInvoker.
VerboseLogPacket(data, len, SCTP_DUMP_INBOUND);
- usrsctp_conninput(this, data, len, 0);
+ usrsctp_conninput(reinterpret_cast<void*>(id_), data, len, 0);
} else {
// TODO(ldixon): Consider caching the packet for very slightly better
// reliability.
@@ -1033,7 +1100,7 @@ sockaddr_conn SctpTransport::GetSctpSockAddr(int port) {
#endif
// Note: conversion from int to uint16_t happens here.
sconn.sconn_port = rtc::HostToNetwork16(port);
- sconn.sconn_addr = this;
+ sconn.sconn_addr = reinterpret_cast<void*>(id_);
return sconn;
}
diff --git a/media/sctp/sctp_transport.h b/media/sctp/sctp_transport.h
index d346cfc71fa8dc106509543110f0f53bd34e61b1..758503b509430ea87825ee9ac023e04b7556ac6f 100644
--- a/media/sctp/sctp_transport.h
+++ b/media/sctp/sctp_transport.h
@@ -13,6 +13,7 @@
#include <errno.h>
+#include <cstdint>
#include <map>
#include <memory>
#include <set>
@@ -267,6 +268,10 @@ class SctpTransport : public SctpTransportInternal,
absl::optional<int> max_outbound_streams_;
absl::optional<int> max_inbound_streams_;
+ // Used for associating this transport with the underlying sctp socket in
+ // various callbacks.
+ uintptr_t id_ = 0;
+
RTC_DISALLOW_COPY_AND_ASSIGN(SctpTransport);
};