Closed Bug 1449038 Opened 6 years ago Closed 3 years ago

Latent use of uninitialized pointer in nsPluginHost::PostURL()

Categories

(Core Graveyard :: Plug-ins, enhancement, P2)

58 Branch
enhancement

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: mozillabugs, Unassigned)

Details

(Keywords: sec-audit)

nsPluginHost::PostURL() (dom\plugins\base\nsPluginHost.cpp) will attempt to use and/or free an uninitialized pointer if it is ever called with |postData == nullptr| . It appears that this cannot occur at present.

The bug is in line 553, which does not check the return status from ParsePostBufferToFixHeaders() (and also on line 551, which doesn't initialize |dataToPost|). Instead, the code relies upon ParsePostBufferToFixHeaders() to initialize |dataToPost| (line 551). However, ParsePostBufferToFixHeaders() does that only if |inPostData != nullptr|.

If PostURL() were to call ParsePostBufferToFixHeaders() with |inPostData == nullptr|, |dataToPost| would remain uninitialized on return. PostURL() would then potentially free it on line 559, or use it to point to a buffer on line 566 (with other code freeing it later):

528: nsresult nsPluginHost::PostURL(nsISupports* pluginInst,
529:                                const char* url,
530:                                uint32_t postDataLen,
531:                                const char* postData,
532:                                const char* target,
533:                                nsNPAPIPluginStreamListener* streamListener,
534:                                const char* altHost,
535:                                const char* referrer,
536:                                bool forceJSEnabled,
537:                                uint32_t postHeadersLength,
538:                                const char* postHeaders)
539: {
540:   nsresult rv;
541: 
542:   // we can only send a stream back to the plugin (as specified
543:   // by a null target) if we also have a nsNPAPIPluginStreamListener
544:   // to talk to also
545:   if (!target && !streamListener)
546:     return NS_ERROR_ILLEGAL_VALUE;
547: 
548:   nsNPAPIPluginInstance* instance = static_cast<nsNPAPIPluginInstance*>(pluginInst);
549: 
550:   nsCOMPtr<nsIInputStream> postStream;
551:   char *dataToPost;
552:   uint32_t newDataToPostLen;
553:   ParsePostBufferToFixHeaders(postData, postDataLen, &dataToPost, &newDataToPostLen);
554:   if (!dataToPost)
555:     return NS_ERROR_UNEXPECTED;
556: 
557:   nsCOMPtr<nsIStringInputStream> sis = do_CreateInstance("@mozilla.org/io/string-input-stream;1", &rv);
558:   if (!sis) {
559:     free(dataToPost);
560:     return rv;
561:   }
562: 
563:   // data allocated by ParsePostBufferToFixHeaders() is managed and
564:   // freed by the string stream.
565:   postDataLen = newDataToPostLen;
566:   sis->AdoptData(dataToPost, postDataLen);
567:   postStream = sis;
568: 
569:   if (target) {
570:     RefPtr<nsPluginInstanceOwner> owner = instance->GetOwner();
571:     if (owner) {
572:       rv = owner->GetURL(url, target, postStream,
573:                          (void*)postHeaders, postHeadersLength, true);
574:     }
575:   }
576: 
577:   // if we don't have a target, just create a stream.
578:   if (streamListener) {
579:     rv = NewPluginURLStream(NS_ConvertUTF8toUTF16(url), instance,
580:                             streamListener,
581:                             postStream, postHeaders, postHeadersLength);
582:   }
583:   return rv;
584: }

3415: nsresult
3416: nsPluginHost::ParsePostBufferToFixHeaders(const char *inPostData, uint32_t inPostDataLen,
3417:                                           char **outPostData, uint32_t *outPostDataLen)
3418: {
3419:   if (!inPostData || !outPostData || !outPostDataLen)
3420:     return NS_ERROR_NULL_POINTER;
3421: 
3422:   *outPostData = 0;
3423:   *outPostDataLen = 0;
3424: 
3425:   const char CR = '\r';
...
3565: }
Group: core-security → dom-core-security
we're unlikely to refactor this code and fall into a trap, we're more likely to remove plugins entirely.

Let's just slap a release assert in here.
Group: dom-core-security
Keywords: sec-audit
Priority: -- → P2
Resolving as wont fix, plugin support deprecated in Firefox 85.
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.