Bug 731917 prevent plugins from corrupting the stack by making word-size stores to pointers to bool r=karlt

dom/plugins/base/nsNPAPIPlugin.cpp

@@ -2020,15 +2020,22 @@ _getvalue(NPP npp, NPNVariable variable, void *result)
       nsNPAPIPluginInstance *inst = (nsNPAPIPluginInstance *) npp->ndata;
       bool windowless = false;
       inst->IsWindowless(&windowless);
-      NPBool needXEmbed = false;
+      // The documentation on the types for many variables in NP(N|P)_GetValue
+      // is vague.  Often boolean values are NPBool (1 byte), but
+      // https://developer.mozilla.org/en/XEmbed_Extension_for_Mozilla_Plugins
+      // treats NPPVpluginNeedsXEmbed as PRBool (int), and
+      // on x86/32-bit, flash stores to this using |movl 0x1,&needsXEmbed|.
+      // thus we can't use NPBool for needsXEmbed, or the three bytes above
+      // it on the stack would get clobbered. so protect with the larger bool.
+      int needsXEmbed = 0;
       if (!windowless) {
-        res = inst->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        res = inst->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needsXEmbed);
         // If the call returned an error code make sure we still use our default value.
         if (NS_FAILED(res)) {
-          needXEmbed = false;
+          needsXEmbed = 0;
         }
       }
-      if (windowless || needXEmbed) {
+      if (windowless || needsXEmbed) {
         (*(Display **)result) = mozilla::DefaultXDisplay();
         return NPERR_NO_ERROR;
       }

dom/plugins/base/nsPluginNativeWindowGtk2.cpp

@@ -120,17 +120,24 @@ nsresult nsPluginNativeWindowGtk2::CallSetWindow(nsRefPtr<nsNPAPIPluginInstance>
       if (!mSocketWidget) {
         nsresult rv;
 
-        bool needXEmbed = false;
+        // The documentation on the types for many variables in NP(N|P)_GetValue
-        rv = aPluginInstance->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        // is vague.  Often boolean values are NPBool (1 byte), but
+        // https://developer.mozilla.org/en/XEmbed_Extension_for_Mozilla_Plugins
+        // treats NPPVpluginNeedsXEmbed as PRBool (int), and
+        // on x86/32-bit, flash stores to this using |movl 0x1,&needsXEmbed|.
+        // thus we can't use NPBool for needsXEmbed, or the three bytes above
+        // it on the stack would get clobbered. so protect with the larger bool.
+        int needsXEmbed = 0;
+        rv = aPluginInstance->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needsXEmbed);
         // If the call returned an error code make sure we still use our default value.
         if (NS_FAILED(rv)) {
-          needXEmbed = false;
+          needsXEmbed = 0;
         }
 #ifdef DEBUG
-        printf("nsPluginNativeWindowGtk2: NPPVpluginNeedsXEmbed=%d\n", needXEmbed);
+        printf("nsPluginNativeWindowGtk2: NPPVpluginNeedsXEmbed=%d\n", needsXEmbed);
 #endif
 
-        if (needXEmbed) {
+        if (needsXEmbed) {
           rv = CreateXEmbedWindow();
         }
         else {

dom/plugins/ipc/PluginInstanceChild.cpp

@@ -625,11 +625,11 @@ PluginInstanceChild::AnswerNPP_GetValue_NPPVpluginNeedsXEmbed(
     // The documentation on the types for many variables in NP(N|P)_GetValue
     // is vague.  Often boolean values are NPBool (1 byte), but
     // https://developer.mozilla.org/en/XEmbed_Extension_for_Mozilla_Plugins
-    // treats NPPVpluginNeedsXEmbed as bool (int), and
+    // treats NPPVpluginNeedsXEmbed as PRBool (int), and
     // on x86/32-bit, flash stores to this using |movl 0x1,&needsXEmbed|.
     // thus we can't use NPBool for needsXEmbed, or the three bytes above
     // it on the stack would get clobbered. so protect with the larger bool.
-    PRUint32 needsXEmbed = 0;
+    int needsXEmbed = 0;
     if (!mPluginIface->getvalue) {
         *rv = NPERR_GENERIC_ERROR;
     }