nsDirIndexParser::OnDataAvailable() (netwerk/streamconv/converters/nsDirIndexParser.cpp) unsafely adds 32-bit integers to calculate an allocation length, potentially causing an overflow, subsequent underallocation, and a write beyond bounds (WBB). The bug appears latent, since the size of the incoming data aCount appears to be limited to the size sent via the chain beginning in HttpBackgroundChannelParent::OnTransportAndData() (netwerk/protocol/http/HttpBackgroundChannelParent.cpp), which calls SendDataInChunks() (netwerk/protocol/http/nsHttp.h), which sends the data to the child process in chunks no larger than 0x20000 bytes.

The bug is on line 357, and the WBB would occur on line 362:

   348: NS_IMETHODIMP
   349: nsDirIndexParser::OnDataAvailable(nsIRequest* aRequest, nsIInputStream* aStream,
   350:                                   uint64_t aSourceOffset, uint32_t aCount) {
   351:   if (aCount < 1) return NS_OK;
   352: 
   353:   int32_t len = mBuf.Length();
   354: 
   355:   // Ensure that our mBuf has capacity to hold the data we're about to
   356:   // read.
   357:   if (!mBuf.SetLength(len + aCount, fallible)) return NS_ERROR_OUT_OF_MEMORY;
   358: 
   359:   // Now read the data into our buffer.
   360:   nsresult rv;
   361:   uint32_t count;
   362:   rv = aStream->Read(mBuf.BeginWriting() + len, aCount, &count);
   ...
   371: }

Depends on D181873

Comment on attachment 9340684 [details]
Bug 1823551 - add overflow check in nsDirIndexParser::OnDataAvailable(). r=#necko

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Based on the patch, it is not that straightforward to create an exploit.
• 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?: No
• If not, how different, hard to create, and risky will they be?: Risk is minimum as we add a guard against overflow.
• How likely is this patch to cause regressions; how much testing does it need?: Less likely
• Is Android affected?: Yes

add overflow check in nsDirIndexParser::OnDataAvailable(). r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/d8d0ea74fa058041a99e6b56e6d28658ed19ce61
https://hg.mozilla.org/mozilla-central/rev/d8d0ea74fa05

Comment on attachment 9340684 [details]
Bug 1823551 - add overflow check in nsDirIndexParser::OnDataAvailable(). r=#necko

Approved to uplift and land

The patch landed in nightly and beta is affected.
:smayya, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox116 to wontfix.

For more information, please visit BugBot documentation.

Comment on attachment 9340684 [details]
Bug 1823551 - add overflow check in nsDirIndexParser::OnDataAvailable(). r=#necko

Beta/Release Uplift Approval Request

• User impact if declined: Overflow could result in undefined behavior.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change adds checks prevent overflow and does not affect user functionality.
• String changes made/needed:
• Is Android affected?: Yes

Comment on attachment 9340684 [details]
Bug 1823551 - add overflow check in nsDirIndexParser::OnDataAvailable(). r=#necko

Approved for 116.0b2

https://hg.mozilla.org/releases/mozilla-beta/rev/985e8c8961c0

Comment on attachment 9340684 [details]
Bug 1823551 - add overflow check in nsDirIndexParser::OnDataAvailable(). r=#necko

Approved for 115.1esr.