Crash report: https://crash-stats.mozilla.org/report/index/7ac7d8cf-9cda-4c10-8329-22a6b0220827

Reason: EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE

Top 10 frames of crashing thread:

0 libsystem_platform.dylib _platform_memmove$VARIANT$Haswell 
1 None @0x000070000f3f870f 
2 XUL parseHashKey netwerk/system/mac/nsNetworkLinkService.mm:318
3 XUL nsNetworkLinkService::calculateNetworkIdInternal netwerk/system/mac/nsNetworkLinkService.mm:596
4 XUL mozilla::detail::RunnableMethodImpl<nsNetworkLinkService*, void  xpcom/threads/nsThreadUtils.h:1200
5 XUL nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:310
6 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1199
7 XUL mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:300
8 XUL MessageLoop::Run ipc/chromium/src/base/message_loop.cc:355
9 XUL nsThread::ThreadFunc xpcom/threads/nsThread.cpp:384

This is a buffer-overflow detected by PHC. I'm marking the bug as security-sensitive because of it though I don't know if it's exploitable. Note that while there doesn't appear to be a memmove() call within the affected code there is a memcpy() one here and it's possible that Apple is using their memmove() implementation for that too under the hood, hence the first frame.

The buffer that is being overflown has being allocated here:

#0    nsNetworkLinkService::calculateNetworkIdInternal() (XUL)
#1    mozilla::detail::RunnableMethodImpl<nsNetworkLinkService*, void (nsNetworkLinkService::*)(), true, (mozilla::RunnableKind)0, >::Run() (XUL)
#2    nsThreadPool::Run() (XUL)
#3    nsThread::ProcessNextEvent(bool, bool*) (XUL)
#4    mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (XUL)
#5    MessageLoop::Run() (XUL)
#6    nsThread::ThreadFunc(void*) (XUL)
#7    _pt_root (libnss3.dylib)
#8    _pthread_start (libsystem_pthread.dylib)
#9    thread_start (libsystem_pthread.dylib)

I think the problem might be at this line.
needed was used as the size of this buffer and it's increased at here. After increasing, needed is already larger than the size of buf.
Then, we copy all data from buf to tmp with needed bytes and we end up reading more bytes than the size of buf.

Comment on attachment 9297247 [details]
Bug 1789808 - Fix buffer overflow, r=#necko

Security Approval Request

• How easily could an exploit be constructed based on the patch?: It's difficult to tell. The patch directly points out the problem, but this code is not triggered every time.
  To hit this code, the errno needs to be ENOMEM.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Should be low, since the patch is straightforward.
• Is Android affected?: No

Hi Dan,

Could you have a look at this sec-approval?
It'd be good if we can land this in this cycle.

Thanks!

Comment on attachment 9297247 [details]
Bug 1789808 - Fix buffer overflow, r=#necko

Approved to land and uplift after beta branches

Fix buffer overflow, r=necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/6c82bce8294f2e8410ec5a4658beb61b84e0c61c
https://hg.mozilla.org/mozilla-central/rev/6c82bce8294f

Comment on attachment 9297247 [details]
Bug 1789808 - Fix buffer overflow, r=#necko

Beta/Release Uplift Approval Request

• User impact if declined: Crash caused by buffer overflow.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: N/A
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a straightforward change. It only affects how we calculate the network id.
• String changes made/needed: N/A
• Is Android affected?: No

Comment on attachment 9297247 [details]
Bug 1789808 - Fix buffer overflow, r=#necko

Approved for 107.0b3.

https://hg.mozilla.org/releases/mozilla-beta/rev/59606feadd11

Please nominate this for ESR102 approval when you get a chance.

Comment on attachment 9297247 [details]
Bug 1789808 - Fix buffer overflow, r=#necko

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: Could crash.
• Fix Landed on Version: 108
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a straightforward patch to avoiding accessing memory out of boundary.

Comment on attachment 9297247 [details]
Bug 1789808 - Fix buffer overflow, r=#necko

Approved for 102.5esr.

https://hg.mozilla.org/releases/mozilla-esr102/rev/4912941eaca3