From 8730dac85fb936308aa6b60c4236083e5cf1123b Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 27 Dec 2009 14:26:00 -0600 Subject: [PATCH] Bug 273025 - "bad logic results in potential leak xor crash based on flow" (Improve GetURL/PostURL code, v4 (2/2)) [r+sr=jst] --- dom/base/nsGlobalWindow.cpp | 11 +-- layout/generic/nsObjectFrame.cpp | 44 ++++----- modules/plugin/base/public/nsIPluginHost.idl | 68 +------------- .../base/public/nsIPluginInstanceOwner.idl | 8 +- .../plugin/base/src/nsNPAPIPluginInstance.cpp | 4 +- modules/plugin/base/src/nsPluginHost.cpp | 91 +++++++++---------- modules/plugin/base/src/nsPluginHost.h | 4 +- 7 files changed, 79 insertions(+), 151 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 7995f34402bf..40da092d2472 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -412,9 +412,9 @@ public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_NSIPLUGININSTANCEOWNER - NS_IMETHOD GetURL(const char *aURL, const char *aTarget, void *aPostData, - PRUint32 aPostDataLen, void *aHeadersData, - PRUint32 aHeadersDataLen, PRBool aIsFile = PR_FALSE); + NS_IMETHOD GetURL(const char *aURL, const char *aTarget, + nsIInputStream *aPostStream, + void *aHeadersData, PRUint32 aHeadersDataLen); NS_IMETHOD ShowStatus(const PRUnichar *aStatusMsg); NPError ShowNativeContextMenu(NPMenu* menu, void* event); NPBool ConvertPoint(double sourceX, double sourceY, NPCoordinateSpace sourceSpace, @@ -494,9 +494,8 @@ nsDummyJavaPluginOwner::CreateWidget(void) NS_IMETHODIMP nsDummyJavaPluginOwner::GetURL(const char *aURL, const char *aTarget, - void *aPostData, PRUint32 aPostDataLen, - void *aHeadersData, PRUint32 aHeadersDataLen, - PRBool isFile) + nsIInputStream *aPostStream, + void *aHeadersData, PRUint32 aHeadersDataLen) { return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/layout/generic/nsObjectFrame.cpp b/layout/generic/nsObjectFrame.cpp index edfa756d199b..189a066502bd 100644 --- a/layout/generic/nsObjectFrame.cpp +++ b/layout/generic/nsObjectFrame.cpp @@ -272,9 +272,9 @@ public: //nsIPluginInstanceOwner interface NS_DECL_NSIPLUGININSTANCEOWNER - NS_IMETHOD GetURL(const char *aURL, const char *aTarget, void *aPostData, - PRUint32 aPostDataLen, void *aHeadersData, - PRUint32 aHeadersDataLen, PRBool isFile = PR_FALSE); + NS_IMETHOD GetURL(const char *aURL, const char *aTarget, + nsIInputStream *aPostStream, + void *aHeadersData, PRUint32 aHeadersDataLen); NS_IMETHOD ShowStatus(const PRUnichar *aStatusMsg); @@ -2608,8 +2608,11 @@ NS_IMETHODIMP nsPluginInstanceOwner::GetInstance(nsIPluginInstance *&aInstance) return NS_OK; } -NS_IMETHODIMP nsPluginInstanceOwner::GetURL(const char *aURL, const char *aTarget, void *aPostData, PRUint32 aPostDataLen, void *aHeadersData, - PRUint32 aHeadersDataLen, PRBool isFile) +NS_IMETHODIMP nsPluginInstanceOwner::GetURL(const char *aURL, + const char *aTarget, + nsIInputStream *aPostStream, + void *aHeadersData, + PRUint32 aHeadersDataLen) { NS_ENSURE_TRUE(mObjectFrame, NS_ERROR_NULL_POINTER); @@ -2634,29 +2637,18 @@ NS_IMETHODIMP nsPluginInstanceOwner::GetURL(const char *aURL, const char *aTarge NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE); - nsCOMPtr postDataStream; nsCOMPtr headersDataStream; + if (aPostStream && aHeadersData) { + if (!aHeadersDataLen) + return NS_ERROR_UNEXPECTED; - // deal with post data, either in a file or raw data, and any headers - if (aPostData) { + nsCOMPtr sis = do_CreateInstance("@mozilla.org/io/string-input-stream;1"); + if (!sis) + return NS_ERROR_OUT_OF_MEMORY; - rv = NS_NewPluginPostDataStream(getter_AddRefs(postDataStream), (const char *)aPostData, aPostDataLen, isFile); - - NS_ASSERTION(NS_SUCCEEDED(rv),"failed in creating plugin post data stream"); - if (NS_FAILED(rv)) - return rv; - - if (aHeadersData) { - rv = NS_NewPluginPostDataStream(getter_AddRefs(headersDataStream), - (const char *) aHeadersData, - aHeadersDataLen, - PR_FALSE, - PR_TRUE); // last arg says we are headers, no /r/n/r/n fixup! - - NS_ASSERTION(NS_SUCCEEDED(rv),"failed in creating plugin header data stream"); - if (NS_FAILED(rv)) - return rv; - } + rv = sis->SetData((char *)aHeadersData, aHeadersDataLen); + NS_ENSURE_SUCCESS(rv, rv); + headersDataStream = do_QueryInterface(sis); } PRInt32 blockPopups = @@ -2664,7 +2656,7 @@ NS_IMETHODIMP nsPluginInstanceOwner::GetURL(const char *aURL, const char *aTarge nsAutoPopupStatePusher popupStatePusher((PopupControlState)blockPopups); rv = lh->OnLinkClick(mContent, uri, unitarget.get(), - postDataStream, headersDataStream); + aPostStream, headersDataStream); return rv; } diff --git a/modules/plugin/base/public/nsIPluginHost.idl b/modules/plugin/base/public/nsIPluginHost.idl index 9a80527aae84..2cf6d9430b95 100644 --- a/modules/plugin/base/public/nsIPluginHost.idl +++ b/modules/plugin/base/public/nsIPluginHost.idl @@ -41,6 +41,7 @@ #include "nsIStreamListener.idl" #include "nsIStringStream.idl" #include "nsIPluginTag.idl" +#include "nsIFile.idl" %{C++ #include "nsPluginNativeWindow.h" @@ -64,7 +65,7 @@ interface nsIPluginStreamListener; [ref] native nsIStreamListenerRef(nsIStreamListener *); [ptr] native nsPluginNativeWindowPtr(nsPluginNativeWindow); -[scriptable, uuid(33211305-4ABD-4E2E-BD73-A05A7DA0E82E)] +[scriptable, uuid(3F3C4952-4877-4D8A-B3CE-A0AFFD2C0785)] interface nsIPluginHost : nsISupports { [noscript] void init(); @@ -242,9 +243,9 @@ interface nsIPluginHost : nsISupports out unsigned long aOutPostDataLen); /** - * To create tmp file with Content len header in, it will use by http POST + * To create temp file with Content len header in, it will use by http POST */ - [noscript] void createTmpFileToPost(in string aPostDataURL, out string aTmpFileName); + [noscript] nsIFile createTempFileToPost(in string aPostDataURL); /** * Creates a new plugin native window object @@ -286,64 +287,3 @@ interface nsIPluginHost : nsISupports virtual void RemoveIdleTimeTarget(nsIPluginInstanceOwner* objectFrame) = 0; %} }; - -%{C++ -#ifdef MOZILLA_INTERNAL_API -/** - * Used for creating the correct input stream for plugins - * We can either have raw data (with or without \r\n\r\n) or a path to a file (but it must be native!) - * When making an nsIInputStream stream for the plugins POST data, be sure to take into - * account that it could be binary and full of nulls, see bug 105417. Also, we need - * to make a copy of the buffer because the plugin may have allocated it on the stack. - * For an example of this, see Shockwave registration or bug 108966 - * We malloc only for headers here, buffer for data itself is malloced by ParsePostBufferToFixHeaders() - */ - -inline nsresult -NS_NewPluginPostDataStream(nsIInputStream **result, - const char *data, - PRUint32 contentLength, - PRBool isFile = PR_FALSE, - PRBool headers = PR_FALSE) -{ - nsresult rv = NS_ERROR_UNEXPECTED; - if (!data) - return rv; - - if (!isFile) { // do raw data case first - if (contentLength < 1) - return rv; - - char *buf = (char*) data; - if (headers) { - // in assumption we got correctly formated headers just passed in - if (!(buf = (char*)nsMemory::Alloc(contentLength))) - return NS_ERROR_OUT_OF_MEMORY; - memcpy(buf, data, contentLength); - } - nsCOMPtr sis = do_CreateInstance("@mozilla.org/io/string-input-stream;1",&rv); - if (NS_SUCCEEDED(rv)) { - sis->AdoptData(buf, contentLength); // let the string stream manage our data - rv = CallQueryInterface(sis, result); - } - else if (headers) { - nsMemory::Free(buf); // Cleanup the memory if the data was copied. - } - } else { - nsCOMPtr file; // tmp file will be deleted on release of stream - nsCOMPtr fileStream; - if (NS_SUCCEEDED(rv = NS_NewNativeLocalFile(nsDependentCString(data), PR_FALSE, getter_AddRefs(file))) && - NS_SUCCEEDED(rv = NS_NewLocalFileInputStream(getter_AddRefs(fileStream), - file, - PR_RDONLY, - 0600, - nsIFileInputStream::DELETE_ON_CLOSE | - nsIFileInputStream::CLOSE_ON_EOF))) { - // wrap the file stream with a buffered input stream - return NS_NewBufferedInputStream(result, fileStream, 8192); - } - } - return rv; -} -#endif -%} diff --git a/modules/plugin/base/public/nsIPluginInstanceOwner.idl b/modules/plugin/base/public/nsIPluginInstanceOwner.idl index 0f2cb23cf7cf..1bf17d4f0743 100644 --- a/modules/plugin/base/public/nsIPluginInstanceOwner.idl +++ b/modules/plugin/base/public/nsIPluginInstanceOwner.idl @@ -38,6 +38,7 @@ #include "nsISupports.idl" #include "nspluginroot.idl" #include "nsIPlugin.idl" +#include "nsIInputStream.idl" interface nsIPluginInstance; interface nsIDocument; @@ -49,7 +50,7 @@ class nsPluginEvent; [ref] native nsIPluginInstanceRef(nsIPluginInstance*); -[uuid(D8776CDC-00DF-4395-A432-2E78EBCC12B6)] +[uuid(B48DC23E-C20B-4292-974E-E8FF97B9F1CC)] interface nsIPluginInstanceOwner : nsISupports { /** @@ -88,9 +89,8 @@ interface nsIPluginInstanceOwner : nsISupports */ NS_IMETHOD GetURL(const char *aURL, const char *aTarget, - void *aPostData, PRUint32 aPostDataLen, - void *aHeadersData, PRUint32 aHeadersDataLen, - PRBool aIsFile = PR_FALSE) = 0; + nsIInputStream *aPostStream, + void *aHeadersData, PRUint32 aHeadersDataLen) = 0; %} /** diff --git a/modules/plugin/base/src/nsNPAPIPluginInstance.cpp b/modules/plugin/base/src/nsNPAPIPluginInstance.cpp index 41b2dd4c5c91..e70f0a53b956 100644 --- a/modules/plugin/base/src/nsNPAPIPluginInstance.cpp +++ b/modules/plugin/base/src/nsNPAPIPluginInstance.cpp @@ -141,7 +141,7 @@ nsPluginStreamToFile::Write(const char* aBuf, PRUint32 aCount, { mOutputStream->Write(aBuf, aCount, aWriteCount); mOutputStream->Flush(); - mOwner->GetURL(mFileURL.get(), mTarget, nsnull, 0, nsnull, 0); + mOwner->GetURL(mFileURL.get(), mTarget, nsnull, nsnull, 0); return NS_OK; } @@ -173,7 +173,7 @@ NS_IMETHODIMP nsPluginStreamToFile::Close(void) { mOutputStream->Close(); - mOwner->GetURL(mFileURL.get(), mTarget, nsnull, 0, nsnull, 0); + mOwner->GetURL(mFileURL.get(), mTarget, nsnull, nsnull, 0); return NS_OK; } diff --git a/modules/plugin/base/src/nsPluginHost.cpp b/modules/plugin/base/src/nsPluginHost.cpp index d46885fa4b49..78645440d13a 100644 --- a/modules/plugin/base/src/nsPluginHost.cpp +++ b/modules/plugin/base/src/nsPluginHost.cpp @@ -2016,13 +2016,13 @@ nsresult nsPluginHost::GetURLWithHeaders(nsISupports* pluginInst, else if (0 == PL_strcmp(target, "_current")) target = "_self"; - rv = owner->GetURL(url, target, nsnull, 0, (void *) getHeaders, getHeadersLength); + rv = owner->GetURL(url, target, nsnull, nsnull, 0); } } if (streamListener) rv = NewPluginURLStream(string, instance, streamListener, nsnull, - PR_FALSE, nsnull, getHeaders, getHeadersLength); + getHeaders, getHeadersLength); return rv; } @@ -2059,22 +2059,44 @@ NS_IMETHODIMP nsPluginHost::PostURL(nsISupports* pluginInst, if (NS_FAILED(rv)) return rv; - char *dataToPost; + nsCOMPtr postStream; if (isFile) { - rv = CreateTmpFileToPost(postData, &dataToPost); - if (NS_FAILED(rv) || !dataToPost) + nsCOMPtr file; + rv = CreateTempFileToPost(postData, getter_AddRefs(file)); + if (NS_FAILED(rv)) + return rv; + + nsCOMPtr fileStream; + rv = NS_NewLocalFileInputStream(getter_AddRefs(fileStream), + file, + PR_RDONLY, + 0600, + nsIFileInputStream::DELETE_ON_CLOSE | + nsIFileInputStream::CLOSE_ON_EOF); + if (NS_FAILED(rv)) + return rv; + + rv = NS_NewBufferedInputStream(getter_AddRefs(postStream), fileStream, 8192); + if (NS_FAILED(rv)) return rv; } else { + char *dataToPost; PRUint32 newDataToPostLen; ParsePostBufferToFixHeaders(postData, postDataLen, &dataToPost, &newDataToPostLen); if (!dataToPost) return NS_ERROR_UNEXPECTED; - // we use nsIStringInputStream::adoptDataa() - // in NS_NewPluginPostDataStream to set the stream - // all new data alloced in ParsePostBufferToFixHeaders() - // well be nsMemory::Free()d on destroy the stream + nsCOMPtr sis = do_CreateInstance("@mozilla.org/io/string-input-stream;1", &rv); + if (!sis) { + NS_Free(dataToPost); + return rv; + } + + // data allocated by ParsePostBufferToFixHeaders() is managed and + // freed by the string stream. postDataLen = newDataToPostLen; + sis->AdoptData(dataToPost, postDataLen); + postStream = sis; } if (target) { @@ -2091,8 +2113,8 @@ NS_IMETHODIMP nsPluginHost::PostURL(nsISupports* pluginInst, target = "_self"; } } - rv = owner->GetURL(url, target, (void*)dataToPost, postDataLen, - (void*)postHeaders, postHeadersLength, isFile); + rv = owner->GetURL(url, target, postStream, + (void*)postHeaders, postHeadersLength); } } @@ -2100,10 +2122,7 @@ NS_IMETHODIMP nsPluginHost::PostURL(nsISupports* pluginInst, // NS_OpenURI()! if (streamListener) rv = NewPluginURLStream(string, instance, streamListener, - (const char*)dataToPost, isFile, postDataLen, - postHeaders, postHeadersLength); - if (isFile) - NS_Free(dataToPost); + postStream, postHeaders, postHeadersLength); return rv; } @@ -4300,9 +4319,7 @@ nsPluginHost::EnsurePrivateDirServiceProvider() nsresult nsPluginHost::NewPluginURLStream(const nsString& aURL, nsIPluginInstance *aInstance, nsIPluginStreamListener* aListener, - const char *aPostData, - PRBool aIsFile, - PRUint32 aPostDataLen, + nsIInputStream *aPostStream, const char *aHeadersData, PRUint32 aHeadersDataLen) { @@ -4353,16 +4370,13 @@ nsresult nsPluginHost::NewPluginURLStream(const nsString& aURL, return NS_ERROR_CONTENT_BLOCKED; } - nsPluginStreamListenerPeer *listenerPeer = new nsPluginStreamListenerPeer; + nsRefPtr listenerPeer = new nsPluginStreamListenerPeer(); if (listenerPeer == NULL) return NS_ERROR_OUT_OF_MEMORY; - NS_ADDREF(listenerPeer); rv = listenerPeer->Initialize(url, aInstance, aListener); - if (NS_FAILED(rv)) { - NS_RELEASE(listenerPeer); + if (NS_FAILED(rv)) return rv; - } nsCOMPtr callbacks; if (doc) { @@ -4402,36 +4416,25 @@ nsresult nsPluginHost::NewPluginURLStream(const nsString& aURL, // deal with headers and post data nsCOMPtr httpChannel(do_QueryInterface(channel)); if (httpChannel) { - if (aPostData) { - nsCOMPtr postDataStream; - rv = NS_NewPluginPostDataStream(getter_AddRefs(postDataStream), (const char*)aPostData, - aPostDataLen, aIsFile); - - if (!postDataStream) { - NS_RELEASE(aInstance); - return NS_ERROR_UNEXPECTED; - } - + if (aPostStream) { // XXX it's a bit of a hack to rewind the postdata stream // here but it has to be done in case the post data is // being reused multiple times. nsCOMPtr - postDataSeekable(do_QueryInterface(postDataStream)); + postDataSeekable(do_QueryInterface(aPostStream)); if (postDataSeekable) postDataSeekable->Seek(nsISeekableStream::NS_SEEK_SET, 0); nsCOMPtr uploadChannel(do_QueryInterface(httpChannel)); NS_ASSERTION(uploadChannel, "http must support nsIUploadChannel"); - uploadChannel->SetUploadStream(postDataStream, EmptyCString(), -1); + uploadChannel->SetUploadStream(aPostStream, EmptyCString(), -1); } if (aHeadersData) rv = AddHeadersToChannel(aHeadersData, aHeadersDataLen, httpChannel); } rv = channel->AsyncOpen(listenerPeer, nsnull); - - NS_RELEASE(listenerPeer); return rv; } @@ -4919,20 +4922,19 @@ nsPluginHost::ParsePostBufferToFixHeaders(const char *inPostData, PRUint32 inPos } NS_IMETHODIMP -nsPluginHost::CreateTmpFileToPost(const char *postDataURL, char **pTmpFileName) +nsPluginHost::CreateTempFileToPost(const char *aPostDataURL, nsIFile **aTmpFile) { - *pTmpFileName = 0; nsresult rv; PRInt64 fileSize; nsCAutoString filename; // stat file == get size & convert file:///c:/ to c: if needed nsCOMPtr inFile; - rv = NS_GetFileFromURLSpec(nsDependentCString(postDataURL), + rv = NS_GetFileFromURLSpec(nsDependentCString(aPostDataURL), getter_AddRefs(inFile)); if (NS_FAILED(rv)) { nsCOMPtr localFile; - rv = NS_NewNativeLocalFile(nsDependentCString(postDataURL), PR_FALSE, + rv = NS_NewNativeLocalFile(nsDependentCString(aPostDataURL), PR_FALSE, getter_AddRefs(localFile)); if (NS_FAILED(rv)) return rv; inFile = localFile; @@ -5014,11 +5016,8 @@ nsPluginHost::CreateTmpFileToPost(const char *postDataURL, char **pTmpFileName) inStream->Close(); outStream->Close(); - if (NS_SUCCEEDED(rv)) { - nsCAutoString path; - if (NS_SUCCEEDED(tempFile->GetNativePath(path))) - *pTmpFileName = ToNewCString(path); - } + if (NS_SUCCEEDED(rv)) + *aTmpFile = tempFile.forget().get(); } return rv; } diff --git a/modules/plugin/base/src/nsPluginHost.h b/modules/plugin/base/src/nsPluginHost.h index 6a7627e1fa9c..06c31d4d3a9b 100644 --- a/modules/plugin/base/src/nsPluginHost.h +++ b/modules/plugin/base/src/nsPluginHost.h @@ -117,9 +117,7 @@ public: NewPluginURLStream(const nsString& aURL, nsIPluginInstance *aInstance, nsIPluginStreamListener *aListener, - const char *aPostData = nsnull, - PRBool isFile = PR_FALSE, - PRUint32 aPostDataLen = 0, + nsIInputStream *aPostStream = nsnull, const char *aHeadersData = nsnull, PRUint32 aHeadersDataLen = 0);