2022079 - Stack-buffer-overflow in LocalAddress::Init()

Status: RESOLVED DUPLICATE of bug 2021770

(Core :: WebRTC: Networking, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: crash_stack.txt

Summary

Stack-buffer-overflow READ in the Firefox Socket Process, reachable from a compromised content process via PMediaTransport::StartIceGathering. Root cause: nr_transport_addr_get_addrstring() (/firefox/dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp:283-284) uses strncpy(str, addr->fqdn, maxlen) without null-terminating the output when strlen(fqdn) >= maxlen. The caller LocalAddress::Init() passes an 80-byte stack buffer, while the IPC deserializer NrIceStunAddr::Deserialize() allows up to 255 non-null bytes in fqdn. The subsequent std::string::operator=(const char*) calls strlen() on the unterminated 80-byte buffer, reading past it.

Affected Code

File: /firefox/dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp, lines 279-308

int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, int maxlen)
  {
    int _status;

    if (addr->fqdn[0]) {
      strncpy(str, addr->fqdn, maxlen);   // BUG: no null-termination when strlen(fqdn) >= maxlen
    } else {
      ...
    }

    _status=0;
  abort:
    return(_status);
  }

strncpy(dst, src, n) does NOT write a null terminator when strlen(src) >= n. The fqdn field is char[256] (transport_addr.h:86); the deserializer (nricestunaddr.cpp:86) only forces fqdn[255]='\0', permitting 255 attacker-controlled non-null bytes. When maxlen=80 and strlen(fqdn)=255, all 80 bytes of str are overwritten with attacker data and no null byte is written.

File: /firefox/dom/media/webrtc/transport/nrinterfaceprioritizer.cpp, lines 27-44

bool Init(const nr_local_addr& local_addr) {
  ifname_ = local_addr.addr.ifname;

  char buf[MAXIFNAME + 47];   // MAXIFNAME=33 -> buf is 80 bytes
  int r = nr_transport_addr_fmt_ifname_addr_string(&local_addr.addr, buf, sizeof(buf));
  // this call DOES null-terminate (transport_addr.cpp:116: buf[len-1]='\0')
  ...
  key_ = buf;  // OK - buf is terminated here

  r = nr_transport_addr_get_addrstring(&local_addr.addr, buf, sizeof(buf));
  // this call OVERWRITES all 80 bytes of buf with 'A' chars,
  // destroying the null terminator, and writes no new one
  ...
  addr_ = buf;  // BUG - std::string::operator=(char*) -> strlen(buf) -> OOB

File: /firefox/dom/media/webrtc/transport/nricestunaddr.cpp, lines 82-86 (insufficient validation)

constexpr size_t fqdn_size =
    sizeof(from_addr->addr.fqdn) / sizeof(from_addr->addr.fqdn[0]);  // 256
from_addr->addr.ifname[ifname_size - 1] = '\0';
from_addr->addr.as_string[as_string_size - 1] = '\0';
from_addr->addr.fqdn[fqdn_size - 1] = '\0';   // only fqdn[255]=0; fqdn[0..254] unchecked

Correct Pattern

if (addr->fqdn[0]) {
  strncpy(str, addr->fqdn, maxlen);
  str[maxlen - 1] = '\0';   // ADD THIS: ensure null termination
}

Note: nr_transport_addr_copy_keep_ifname() at line 172-173 in the same file already demonstrates the correct pattern: strncpy(save_ifname, to->ifname, MAXIFNAME); save_ifname[MAXIFNAME-1]=0; /* Ensure null termination */ — the developers knew the idiom but missed this call site.

Exploit Chain

1. Attacker compromises a content process (assumed precondition).
2. Content process instantiates RTCPeerConnection and calls setLocalDescription(), which invokes MediaTransportHandlerIPC::StartIceGathering() (content-only class; created at MediaTransportHandler.cpp:201 under XRE_IsContentProcess()).
3. Attacker crafts an nsTArray<NrIceStunAddr> with one entry: ip_version=NR_IPV4, sin_family=AF_INET, sin_addr=10.0.0.1 (passes loopback/link-local filters), ifname="eth0", and fqdn[0..255] all set to 'A'.
4. Content sends PMediaTransport::SendStartIceGathering(defaultRouteOnly=false, obfuscate=false, [evil_addr]). NrIceStunAddr::Serialize() → nr_local_addr_copy() → nr_transport_addr_copy() = raw memcpy(sizeof(nr_transport_addr)) — all 256 fqdn bytes cross the wire intact.
5. Socket process: MediaTransportParent::RecvStartIceGathering() → NrIceStunAddr::Deserialize() sets fqdn[255]='\0' (yielding strlen(fqdn)==255) and passes validation.
6. MediaTransportHandlerSTS::StartIceGathering() (MediaTransportHandler.cpp:921) → NrIceCtx::SetStunAddrs() (nricectx.cpp:677) → nr_local_addr_copy() (memcpy, fqdn preserved) → nr_ice_set_local_addresses().
7. nr_ice_set_local_addresses() (ice_ctx.cpp): nr_stun_filter_addrs() accepts the address (IPv4, not loopback, not link-local). Because defaultRouteOnly=false, NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS is clear, so the else { addrs = local_addrs; } branch is taken (line 791), reaching the prioritizer loop at line 798.
8. nr_interface_prioritizer_add_interface() → add_interface() → InterfacePrioritizer::add() → LocalAddress::Init().
9. char buf[80]; nr_transport_addr_get_addrstring(&addr, buf, 80) → fqdn[0]=='A' is truthy → strncpy(buf, fqdn, 80) copies 80 'A' bytes, writes no '\0'.
10. addr_ = buf → std::basic_string::operator=(const char*) → std::char_traits::length() → strlen(buf) reads buf[0..79] (all 'A') then buf[80] — stack OOB read.

IPC Path

• IPDL protocol: PMediaTransport (carried over PSocketProcessBridge)
• Trust boundary: /firefox/netwerk/ipc/PSocketProcessBridge.ipdl:26 and /firefox/dom/media/webrtc/PMediaTransport.ipdl:32 — both declare [ParentProc=Socket, ChildProc=Content]
• Parent actor: MediaTransportParent (Socket Process) — /firefox/dom/media/webrtc/jsapi/MediaTransportParent.cpp:207 RecvStartIceGathering
• Child actor: MediaTransportChild via MediaTransportHandlerIPC (Content Process)
• Message: async StartIceGathering(bool defaultRouteOnly, bool obfuscateHostAddresses, NrIceStunAddrArray stunAddrs) — sent to parent:
• Tainted data: NrIceStunAddr.localAddr_.addr.fqdn (256 bytes) — crosses via memcpy-based serialization with only last-byte null-termination as validation

Security Impact

Severity: Moderate-to-High (stack info leak primitive in a more-privileged process)

Attacker capability: A compromised content process can cause the Socket Process to over-read its own stack into a heap-allocated std::string. In a release (non-ASAN) build, strlen() will continue reading stack memory past buf[79] until it encounters a null byte; the over-read bytes (potentially including return addresses, saved registers, stack canaries, or other locals depending on compiler stack layout) are then copied to the heap as part of addr_. This std::string is inserted into a std::set<LocalAddress> and used for preference sorting; the sort order (observable via the order of ICE candidates returned to content) could leak partial information about the over-read bytes. This is an information-disclosure / ASLR-defeat primitive, not a direct write primitive.

Preconditions:

• network.process.enabled=true (default on desktop Firefox)
• media.peerconnection.mtransport_process=true (default on desktop Firefox)
• Attacker has already achieved arbitrary code execution in a content process

Sandbox escape direction: Content Process (heavily sandboxed, no network) → Socket Process (has network access). This crosses a genuine security boundary.

ASAN Report

==207485==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7b62740e4070 at pc 0x64a04f326763 bp 0x7b6274ee0bd0 sp 0x7b6274ee0398
READ of size 81 at 0x7b62740e4070 thread T5
    #0 0x64a04f326762 in ___interceptor_strlen /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:425:5
    #1 0x7b628da617d2 in std::char_traits<char>::length(char const*) .../char_traits.h:399:9
    #2 0x7b628da617d2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::assign(char const*) .../basic_string.h:1452:6
    #3 0x7b628da617d2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::operator=(char const*) .../basic_string.h:689:22
    #4 0x7b628da617d2 in (anonymous namespace)::LocalAddress::Init(nr_local_addr_ const&) /firefox/dom/media/webrtc/transport/nrinterfaceprioritizer.cpp:44:11
    #5 0x7b628da617d2 in (anonymous namespace)::InterfacePrioritizer::add(nr_local_addr_ const*) /firefox/dom/media/webrtc/transport/nrinterfaceprioritizer.cpp:170:15
    #6 0x7b628da617d2 in add_interface(void*, nr_local_addr_*) /firefox/dom/media/webrtc/transport/nrinterfaceprioritizer.cpp:217:14
    #7 0x7b629ad11a97 in nr_ice_set_local_addresses(nr_ice_ctx_*, nr_local_addr_*, int) /firefox/dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_ctx.cpp:798:15
    #8 0x7b628da3b405 in mozilla::NrIceCtx::SetStunAddrs(nsTArray<mozilla::NrIceStunAddr> const&) /firefox/dom/media/webrtc/transport/nricectx.cpp:677:3
    #9 0x7b629597841e in mozilla::MediaTransportHandlerSTS::StartIceGathering(bool, bool, nsTArray<mozilla::NrIceStunAddr> const&)::$_0::operator()() const /firefox/dom/media/webrtc/jsapi/MediaTransportHandler.cpp:921:20
    ...
    #16 0x7b628bedea11 in mozilla::net::nsSocketTransportService::Run() /firefox/netwerk/base/nsSocketTransportService2.cpp:1259:11

Address 0x7b62740e4070 is located in stack of thread T5 at offset 112 in frame
    #0 0x7b628da613e7 in add_interface(void*, nr_local_addr_*) /firefox/dom/media/webrtc/transport/nrinterfaceprioritizer.cpp:215:0

  This frame has 6 object(s):
    [32, 112) 'buf.i.i' (line 30)
    [144, 536) 'str.i.i' (line 34) <== Memory access at offset 112 partially underflows this variable
    [608, 640) 'ref.tmp.i.i' (line 34)
    [672, 1064) 'str59.i.i' (line 41)
    [1136, 1168) 'ref.tmp78.i.i' (line 41)
    [1200, 1312) 'addr.i' (line 169)

Thread T5 created by T0 (Socket Process) here:
    ...
    #28 0x7b628d044f2d in mozilla::net::SocketProcessChild::Init(mozilla::ipc::UntypedEndpoint&&, char const*) /firefox/netwerk/ipc/SocketProcessChild.cpp:192:32
    #29 0x7b628d052847 in mozilla::net::SocketProcessImpl::Init(int, char**) /firefox/netwerk/ipc/SocketProcessImpl.cpp:64:31
    #30 0x7b629ab15b5a in XRE_InitChildProcess(int, char**, XREChildData const*) /firefox/toolkit/xre/nsEmbedFunctions.cpp:561:21
    #31 0x64a04f3f3e7a in main /firefox/browser/app/nsBrowserApp.cpp:467:22

SUMMARY: AddressSanitizer: stack-buffer-overflow (/firefox/obj-x86_64-pc-linux-gnu/dist/bin/firefox+0xef762)
Shadow bytes around the buggy address:
=>0x7b62740e4000: f1 f1 f1 f1 00 00 00 00 00 00 00 00 00 00[f2]f2

Suggested Fix

Primary fix — /firefox/dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp:283-284:

if (addr->fqdn[0]) {
  strncpy(str, addr->fqdn, maxlen);
  str[maxlen - 1] = '\0';   // Ensure null termination (matches lines 116, 173 in same file)
}

Defense-in-depth — /firefox/dom/media/webrtc/transport/nricestunaddr.cpp:86: In addition to fqdn[255]='\0', the deserializer should validate that fqdn is either empty or contains a reasonable hostname length. No legitimate STUN local-address gathering produces a 255-byte fqdn for a local interface; rejecting strlen(fqdn) > 46 (max IPv6 string length) would close this and any similar gadget reachable via the same field.

Comment 1

[Tyson Smith [:tsmith]]

Attachment: exploit.patch

Comment 2

[Tyson Smith [:tsmith]]

Attachment: test.html

Comment 3

[Byron Campen [:bwc]]

This seems to be a dupe of bug 2021770, do you concur?

Comment 4

[Tyson Smith [:tsmith]]

(In reply to Byron Campen [:bwc] from comment #3)

This seems to be a dupe of bug 2021770, do you concur?

Yes. I am not able to reproduce this issue with the patch from bug 2021770 applied, thanks!