Closed Bug 2021770 (CVE-2026-6776) Opened 8 months ago Closed 8 months ago

Missing NUL Termination in nr_transport_addr_get_addrstring Leads to Stack Buffer Overflow Read

Categories

(Core :: WebRTC: Networking, defect)

defect

Tracking

()

RESOLVED FIXED
150 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox-esr140 150+ fixed
firefox148 --- wontfix
firefox149 --- wontfix
firefox150 + fixed

People

(Reporter: sakura, Assigned: bwc)

References

Details

(Keywords: csectype-bounds, reporter-external, sec-low, Whiteboard: [client-bounty-form][adv-main150+][adv-esr140.10+])

Attachments

(2 files, 2 external files)

Attached file poc.html —

Missing NUL Termination in nr_transport_addr_get_addrstring Leads to Stack Buffer Overflow Read

Summary

The nICEr library function nr_transport_addr_get_addrstring uses strncpy to copy an FQDN hostname into a caller-provided buffer without ensuring NUL termination. When a web page provides a TURN or STUN server with a hostname of 64 bytes or more (the size of the destination buffer in the calling function nr_transport_addr_get_addrstring_and_port), strncpy fills the buffer without appending a trailing NUL byte. The subsequent assignment to an nsCString reads past the buffer boundary until encountering a NUL byte elsewhere on the stack. This affects all platforms. No special preferences are needed beyond the default WebRTC configuration.

Affected Versions

The vulnerable code was introduced in commit 13b16b7d0e02 on 2021-01-28 (Bug 1416220: Modify mtransport to delegate DNS lookups for TCP sockets to the socket class, instead of using nr_resolver). That commit added the fqdn field to nr_transport_addr and the strncpy call in nr_transport_addr_get_addrstring without NUL termination. All Firefox versions from that point onward are affected. Testing was performed on mozilla-central commit 9deea8b0a7f3 (Firefox 150.0a1, Nightly build 20260307210637).

Root Cause

The function nr_transport_addr_get_addrstring, located in transport_addr.cpp at lines 279 through 308, handles the FQDN case with a bare strncpy call:

// dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp:283-284
if (addr->fqdn[0]) {
    strncpy(str, addr->fqdn, maxlen);
}

The C library function strncpy does not NUL-terminate the destination when the source length is greater than or equal to maxlen. The fqdn field is declared as char[256] in transport_addr.h at line 86, while the caller nr_transport_addr_get_addrstring_and_port declares a local char addr_string[64] buffer:

// dom/media/webrtc/transport/nr_socket_prsock.cpp:528-545
int nr_transport_addr_get_addrstring_and_port(const nr_transport_addr* addr,
                                              nsACString* host, int32_t* port) {
  int r, _status;
  char addr_string[64];
  if ((r = nr_transport_addr_get_addrstring(addr, addr_string,
                                            sizeof(addr_string)))) {
    ABORT(r);
  }
  ...
  *host = addr_string;

The assignment *host = addr_string treats addr_string as a C string and internally calls strlen to determine the length. When the FQDN is 64 bytes or longer, addr_string has no NUL terminator, and strlen reads past the 64-byte buffer into adjacent stack memory.

JavaScript can provide hostnames up to 255 bytes through the RTCPeerConnection iceServers configuration. The NrIceStunServer::Init() function accepts hostnames with a size less than 256. During ICE gathering for TURN/TCP or TURNS servers, the hostname is preserved in the fqdn field rather than being DNS-resolved in the socket process. When the socket process subsequently attempts to connect, nr_transport_addr_get_addrstring_and_port is called with the oversized FQDN, triggering the overflow read.

Reproduce

Testing was performed on Firefox Nightly 150.0a1 (BuildID 20260307210637) from mozilla-central revision 9deea8b0a7f33f6410ce3ee1b718a9bc3b394ea5, downloaded via fuzzfetch as an ASAN-optimized build for Linux x86-64.

No source modifications are required. The proof of concept (poc.html) constructs two RTCPeerConnections with TURN server hostnames exceeding 64 bytes. The first uses the turns: scheme (TLS over TCP) and the second uses turn:?transport=tcp, both of which take the NrTcpSocket code path that preserves the FQDN. The hostname is built from two 63-character DNS labels plus ".example.com", totaling 139 bytes. After creating data channels and setting local descriptions, ICE gathering triggers the vulnerable code path.

To reproduce, serve the PoC over HTTP from the directory containing poc.html:

python3 -m http.server 8080

No special preferences are needed; all required functionality (WebRTC, ICE gathering) is enabled by default. Create a temporary profile and launch Firefox:

TMPDIR=$(mktemp -d)
ASAN_OPTIONS="detect_leaks=0" \
  ./firefox --no-remote \
  --profile "$TMPDIR" \
  http://localhost:8080/poc.html

The crash occurs within a few seconds during ICE gathering in the Socket Process thread (T5).

The ASAN output from the fuzzfetch build (full log available in asan.log) is as follows:

==2716710==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7b791100cee0 at pc 0x55af84ab7433 bp 0x7b79121fd590 sp 0x7b79121fcd58
READ of size 65 at 0x7b791100cee0 thread T5
    #0 strlen sanitizer_common_interceptors.inc:425:5
    #1 nsTSubstring<char>::Assign nr_socket_prsock.cpp via nsCharTraits.h:425:56
    #2 nsTSubstring<char>::Assign nsTSubstring.cpp:397:7
    #3 operator= nsTSubstring.h:515:5
    #4 nr_transport_addr_get_addrstring_and_port nr_socket_prsock.cpp:545:9
    #5 NrTcpSocket::connect nr_socket_tcp.cpp:103:7
    #6 nr_socket_buffered_stun_connect nr_socket_buffered_stun.cpp:452:10
    #7 nr_socket_multi_tcp_create_stun_server_socket nr_socket_multi_tcp.cpp:199:9
    #8 nr_socket_multi_tcp_create nr_socket_multi_tcp.cpp:276:18
    #9 nr_ice_component_create_tcp_host_candidate ice_component.cpp:393:9
    #10 nr_ice_component_initialize_tcp ice_component.cpp:478:16
    #11 nr_ice_component_initialize ice_component.cpp:674:11
    #12 nr_ice_media_stream_initialize ice_media_stream.cpp:176:12
    #13 nr_ice_gather ice_ctx.cpp:869:14
    #14 NrIceCtx::StartGathering nricectx.cpp:974:11

Address 0x7b791100cee0 is located in stack of thread T5 at offset 96 in frame
    #0 nr_transport_addr_get_addrstring_and_port nr_socket_prsock.cpp:529

  This frame has 1 object(s):
    [32, 96) 'addr_string' (line 531) <== Memory access at offset 96 overflows this variable

SUMMARY: AddressSanitizer: stack-buffer-overflow nsCharTraits.h:425:56 in length

Fix

The fix should ensure NUL termination after the strncpy call in the FQDN branch. Since the function does not return an error when the FQDN is too long for the buffer, it should also be updated to fail if the FQDN does not fit:

--- a/dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp
+++ b/dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp
@@ -280,7 +280,10 @@ int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, i
     int _status;

     if (addr->fqdn[0]) {
-      strncpy(str, addr->fqdn, maxlen);
+      size_t fqdn_len = strlen(addr->fqdn);
+      if (fqdn_len >= (size_t)maxlen)
+        ABORT(R_BAD_ARGS);
+      memcpy(str, addr->fqdn, fqdn_len + 1);
     } else {
       const char* res;
       switch (addr->ip_version) {

This replaces the strncpy with an explicit length check and memcpy that always includes the NUL terminator. If the FQDN is too long for the provided buffer, the function returns R_BAD_ARGS rather than silently producing an unterminated string.

Flags: sec-bounty?
Attached file asan.log —
Group: firefox-core-security → media-core-security
Component: Security → WebRTC: Networking
Product: Firefox → Core
Attachment #9550867 - Attachment mime type: application/octet-stream → text/plain
Assignee: nobody → docfaraday
Attached file (secure) —

Also, make nr_transport_addr_get_addrstring return an error if the
caller does not pass a large enough buffer, instead of just truncating
the output.

So this is an OOB read on the socket process, but I am not seeing how any of that read would be exfiltrated. Are we sure this is exploitable?

Flags: needinfo?(continuation)
Flags: needinfo?(continuation)
Keywords: sec-high → sec-moderate
See Also: → 2022079
Duplicate of this bug: 2022079
Pushed by nfay@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/764d9abb49f5 https://hg.mozilla.org/integration/autoland/rev/a8d287d9bcd8 Revert "Bug 2021770: Make this general-purpose helper support big FQDNs r=mjf" for causing bustage @ transport_addr.cpp
[task 2026-03-12T22:48:50.655+00:00] 22:48:50    ERROR -  /builds/worker/checkouts/gecko/dom/media/webrtc/transport/third_party/nICEr/src/net/transport_addr.cpp:284:18: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
[task 2026-03-12T22:48:50.655+00:00] 22:48:50     INFO -    284 |       if (maxlen <= strlen(addr->fqdn)) {
Flags: needinfo?(docfaraday)

Bah, my try push was before we converted those sources to c++, apparently. We might need to modify this slightly for uplift.

Flags: needinfo?(docfaraday)
Group: media-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 150 Branch

Go ahead and attach Beta and ESR140 uplift patches when you have them ready.

Byron is out this week. Let's let this ride Fx150 and aim for an ESR140 uplift request for 140.10esr next week.

firefox-esr140 Uplift Approval Request

  • User impact if declined/Reason for urgency: Moderate sec bug.
  • Code covered by automated testing?: no
  • Fix verified in Nightly?: yes
  • Needs manual QE testing?: no
  • Steps to reproduce for manual QE testing:
  • Risk associated with taking this patch: low
  • Explanation of risk level: This just adds some bounds-checking, and should not have any side-effects.
  • String changes made/needed?: None
  • Is Android affected?: yes
Attachment #9558165 - Flags: approval-mozilla-esr140?
Attached file (secure) —

Also, make nr_transport_addr_get_addrstring return an error if the
caller does not pass a large enough buffer, instead of just truncating
the output.

Original Revision: https://phabricator.services.mozilla.com/D286797

Nan: the over-read is a real bug, but we can't see how to turn this into a vulnerability beyond denial of service. In order to consider this for a bug bounty we need evidence of some way this benefits the attacker such as returning socket process memory that maybe could be used to aim the next vulnerability in an exploit chain

Flags: sec-bounty? → sec-bounty-
Keywords: sec-moderate → sec-low
QA Whiteboard: [sec] [qa-triage-done-c151/b150]

Once we get the beta uplift here, I can request an ESR 140 uplift using that patch (I think? Will Phabricator be sad if I request uplift on the patch fixed for beta, instead of the original?).

Edit: Nevermind, was thinking of one of the many other sec bugs.

Flags: needinfo?(docfaraday)
Attachment #9558165 - Flags: approval-mozilla-beta+
Attachment #9558165 - Flags: approval-mozilla-esr140? → approval-mozilla-esr140+
Attachment #9558165 - Flags: approval-mozilla-beta+
Whiteboard: [client-bounty-form] → [client-bounty-form][adv-main150+]
Whiteboard: [client-bounty-form][adv-main150+] → [client-bounty-form][adv-main150+][adv-esr140.10+]
Alias: CVE-2026-6776
Flags: sec-bounty-hof+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: