Bug 731917 prevent plugins from corrupting the stack by making word-size stores to pointers to bool r=karlt
authorGinn Chen <ginn.chen@oracle.com>
Fri, 02 Mar 2012 14:17:26 +0800
changeset 88137 02dcec80742225f93746f18f252bd43d71f1c2aa
parent 88136 c11552388c8bedab9bbb982938d0c41acddbf2a7
child 88138 21ce98516c3a1215af4931521a4a7b52bf2e1261
push id22171
push usermak77@bonardo.net
push dateFri, 02 Mar 2012 13:56:30 +0000
treeherderautoland@343ec916dfd5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskarlt
bugs731917
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 731917 prevent plugins from corrupting the stack by making word-size stores to pointers to bool r=karlt
dom/plugins/base/nsNPAPIPlugin.cpp
dom/plugins/base/nsPluginNativeWindowGtk2.cpp
dom/plugins/ipc/PluginInstanceChild.cpp
--- a/dom/plugins/base/nsNPAPIPlugin.cpp
+++ b/dom/plugins/base/nsNPAPIPlugin.cpp
@@ -2015,25 +2015,32 @@ NPError NP_CALLBACK
   switch(variable) {
 #if defined(XP_UNIX) && !defined(XP_MACOSX)
   case NPNVxDisplay : {
 #if defined(MOZ_X11)
     if (npp) {
       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;
       }
     }
 #ifdef MOZ_WIDGET_GTK2
     // adobe nppdf calls XtGetApplicationNameAndClass(display,
     // &instance, &class) we have to init Xt toolkit before get
     // XtDisplay just call gtk_xtbin_new(w,0) once
--- a/dom/plugins/base/nsPluginNativeWindowGtk2.cpp
+++ b/dom/plugins/base/nsPluginNativeWindowGtk2.cpp
@@ -115,27 +115,34 @@ nsresult PLUG_DeletePluginNativeWindow(n
 
 nsresult nsPluginNativeWindowGtk2::CallSetWindow(nsRefPtr<nsNPAPIPluginInstance> &aPluginInstance)
 {
   if (aPluginInstance) {
     if (type == NPWindowTypeWindow) {
       if (!mSocketWidget) {
         nsresult rv;
 
-        bool needXEmbed = false;
-        rv = aPluginInstance->GetValueFromPlugin(NPPVpluginNeedsXEmbed, &needXEmbed);
+        // 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;
+        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 {
           rv = CreateXtWindow();
         }
 
         if (NS_FAILED(rv)) {
           return NS_ERROR_FAILURE;
--- a/dom/plugins/ipc/PluginInstanceChild.cpp
+++ b/dom/plugins/ipc/PluginInstanceChild.cpp
@@ -620,21 +620,21 @@ PluginInstanceChild::AnswerNPP_GetValue_
     bool* needs, NPError* rv)
 {
     AssertPluginThread();
 
 #ifdef MOZ_X11
     // 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;
     }
     else {
         *rv = mPluginIface->getvalue(GetNPP(), NPPVpluginNeedsXEmbed,
                                      &needsXEmbed);
     }
     *needs = needsXEmbed;