As part of a code review I am conducting for Paul Theriault against the sandbox, the following vulnerability has been discovered. A compromised content process can initiate an FTP file download which will in turn generate a mozilla::net::PFTPChannelChild object to handle downloading the files contents for rendering in the content process. If the MIME type is not supported for rendering in the content process, the file will be passed over to the Chrome process so the user may elect to either open or download the file. When the file is to be passed over to the Chrome process, the child content process will call FTPChannelParent::RecvDivertOnDataAvailable in the parent chrome process via the PFTPChannel IPDL protocol. The child may supply a length value which is greater than the total size of the file to be downloaded via the 'count' parameter. > mozilla::ipc::IPCResult > FTPChannelParent::RecvDivertOnDataAvailable(const nsCString& data, // <-- a byte buffer holding the files contents. > const uint64_t& offset, > const uint32_t& count) // <-- attacker can supply a count value greater than the size of the above data buffer. > { > if (NS_WARN_IF(!mDivertingFromChild)) { > MOZ_ASSERT(mDivertingFromChild, > "Cannot RecvDivertOnDataAvailable if diverting is not set!"); > FailDiversion(NS_ERROR_UNEXPECTED); > return IPC_FAIL_NO_REASON(this); > } > > // Drop OnDataAvailables if the parent was canceled already. > if (NS_FAILED(mStatus)) { > return IPC_OK(); > } > > mEventQ->RunOrEnqueue(new FTPDivertDataAvailableEvent(this, data, offset, > count)); > return IPC_OK(); > } The FTPDivertDataAvailableEvent event will in turn call FTPChannelParent::DivertOnDataAvailable which will create an new nsIInputStream to represent the data buffer to process. This stream can have an arbitrary length value supplied by the attacker. If the length value is greater than the data buffer supplied by the attacker, then the adjacent contents of memory can be read from when OnDataAvailable processes the stream. > void > FTPChannelParent::DivertOnDataAvailable(const nsCString& data, > const uint64_t& offset, > const uint32_t& count) > { > LOG(("FTPChannelParent::DivertOnDataAvailable [this=%p]\n", this)); > > if (NS_WARN_IF(!mDivertingFromChild)) { > MOZ_ASSERT(mDivertingFromChild, > "Cannot DivertOnDataAvailable if diverting is not set!"); > FailDiversion(NS_ERROR_UNEXPECTED); > return; > } > > // Drop OnDataAvailables if the parent was canceled already. > if (NS_FAILED(mStatus)) { > return; > } > > nsCOMPtr<nsIInputStream> stringStream; > nsresult rv = NS_NewByteInputStream(getter_AddRefs(stringStream), data.get(), // <-- a pointer to the start or the data bufffer > count, NS_ASSIGNMENT_DEPEND); // <-- the count value which may be > then the size of the data buffer > if (NS_FAILED(rv)) { > if (mChannel) { > mChannel->Cancel(rv); > } > mStatus = rv; > return; > } > > AutoEventEnqueuer ensureSerialDispatch(mEventQ); > > rv = OnDataAvailable(mChannel, nullptr, stringStream, offset, count); // <-- continue to process the data and download it to disk. > > stringStream->Close(); > if (NS_FAILED(rv)) { > if (mChannel) { > mChannel->Cancel(rv); > } > mStatus = rv; > } > } Firefox's default action is to present the user with a dialog to either open or save the file. However in the background the contents of the file will be downloaded to a temp directory with no user interaction required. For example on a Win10 machine I get a file like "C:\Users\stephen\AppData\Local\Temp\qfamDcQE.zip.part". This location will obviously be different across OS platforms which may effect how easily an attacker can read it back depending on the sandbox implementation for that OS. If an attacker can read this files contents - * They would need read permission explicitly for this file and then either discover or brute force the 'qfamDcQE' part of the file name. * They would need read permissions on the temp directory. Then discovering the file name to read should be trivial as directory enumeration API access should be allowed (e.g FindFirstfile/FindNextFile on Windows). The attacker can now leak out memory from the Chrome process. This could be leveraged by an attacker to either bypass ASLR for an additional memory corruption based sandbox escape exploit, or to try and leak out sensitive contents from the Chrome process (Perhaps session keys or similar). To reproduce this issue: * Visit ftp://speedtest.tele2.net/ in a content process. * Optional: Run a tool like procmon.exe to monitor firefox.exe file access if you want to see the .zip.part file being written. * Attach WinDbg to this content process. * Break into WinDbg and set a breakpoint via the command "bp xul!mozilla::net::PFTPChannelChild::SendDivertOnDataAvailable". Now continue execution of the process. * Click a file to download, for example ftp://speedtest.tele2.net/1KB.zip which is 1024 (0x400) bytes in size. * WinDbg will break in SendDivertOnDataAvailable. We now simulate a compromised content process. Using the WinDbg Locals window, edit the count value from 0x400 to something larger like 0x1400. Now press F5 to continue execution. * Firefox will display a save/open dialog, you can ignore this. * The "XXXXXXXX.zip.part" file will automatically be written to disk in the background by the Chrome process. It will be written to your AppData\Local\Temp\ directory. Inspecting this file will show the file size is 0x1400 and at offset 0x400 (the original file size) are 0x1000 bytes of the Chrome processes memory which was adjacent to the original data buffer from the original FTPChannelParent::RecvDivertOnDataAvailable call. * Note: There is a chance the Chrome process will read into unallocated memory and access violate. The attacker would need to perform some form of heap massaging in order to influence the layout of the heap and ensure this doesn't happen and that they leak some thing interesting.

(In reply to Stephen Fewer from comment #0) > > nsresult rv = NS_NewByteInputStream(getter_AddRefs(stringStream), data.get(), // <-- a pointer to the start or the data bufffer > > count, NS_ASSIGNMENT_DEPEND); // <-- the count value which may be > then the size of the data buffer cropping |count| as min(count, data.Length()) is the solution here.

Should the |min| call be done there, or when we |SendDivertOnDataAvailable| (or maybe even before then)? I'm not familiar with this code enough to say for certain which makes sense.