GNOME.org
 

[totem] Fix plugin crash in webkit browsers

commit e7b035dd531406066f75ee9f28c4ba56ad667464
Author: Christian Persch <chpe gnome org>
Date:   Sat Sep 12 23:07:52 2009 +0200

    Fix plugin crash in webkit browsers
    
    Don't assume the strings from NPAPI are 0-terminated.
    Bug #537456.

 browser-plugin/npapi.h                    |    1 +
 browser-plugin/totemConePlaylist.cpp      |    4 +-
 browser-plugin/totemGMPControls.cpp       |   16 ++++----
 browser-plugin/totemGMPPlayer.cpp         |    4 +-
 browser-plugin/totemNPNGlue.cpp           |   25 ++++++++++++-
 browser-plugin/totemNPObject.cpp          |   22 +++++------
 browser-plugin/totemNPObject.h            |    2 +-
 browser-plugin/totemNarrowSpacePlugin.cpp |   16 ++++----
 browser-plugin/totemPlugin.cpp            |   55 +++++++++++++++++++++-------
 browser-plugin/totemPlugin.h              |   11 +++---
 10 files changed, 102 insertions(+), 54 deletions(-)
---
diff --git a/browser-plugin/npapi.h b/browser-plugin/npapi.h
index 952d357..5ae09f7 100644
--- a/browser-plugin/npapi.h
+++ b/browser-plugin/npapi.h
@@ -784,6 +784,7 @@ void        NPN_MemFree(void* ptr);
 uint32        NPN_MemFlush(uint32 size);
 void* NPN_MemDup (const void*, uint32);
 char* NPN_StrDup (const char *);
+char* NPN_StrnDup (const char *, uint32);
 void        NPN_ReloadPlugins(NPBool reloadPages);
 JRIEnv*     NPN_GetJavaEnv(void);
 jref        NPN_GetJavaPeer(NPP instance);
diff --git a/browser-plugin/totemConePlaylist.cpp b/browser-plugin/totemConePlaylist.cpp
index 7877a6f..ac53f85 100644
--- a/browser-plugin/totemConePlaylist.cpp
+++ b/browser-plugin/totemConePlaylist.cpp
@@ -75,8 +75,8 @@ totemConePlaylist::InvokeByIndex (int aIndex,
       if (!CheckArgc (argc, 1, 3))
         return false;
 
-      const char *mrl;
-      if (!GetStringFromArguments (argv, argc, 0, mrl))
+      NPString mrl;
+      if (!GetNPStringFromArguments (argv, argc, 0, mrl))
         return false;
 
       Plugin()->AddItem (mrl);
diff --git a/browser-plugin/totemGMPControls.cpp b/browser-plugin/totemGMPControls.cpp
index 96f00bb..4166550 100644
--- a/browser-plugin/totemGMPControls.cpp
+++ b/browser-plugin/totemGMPControls.cpp
@@ -108,15 +108,15 @@ totemGMPControls::InvokeByIndex (int aIndex,
 
     case eIsAvailable:
       /* boolean isAvailable (in ACString name); */
-      const char *name;
-      if (!GetStringFromArguments (argv, argc, 0, name))
+      NPString name;
+      if (!GetNPStringFromArguments (argv, argc, 0, name))
         return false;
-      if (g_ascii_strcasecmp (name, "currentItem") == 0
-      	  || g_ascii_strcasecmp (name, "next") == 0
-      	  || g_ascii_strcasecmp (name, "pause") == 0
-      	  || g_ascii_strcasecmp (name, "play") == 0
-      	  || g_ascii_strcasecmp (name, "previous") == 0
-      	  || g_ascii_strcasecmp (name, "stop") == 0)
+      if (g_ascii_strncasecmp (name.UTF8Characters, "currentItem", name.UTF8Length) == 0
+      	  || g_ascii_strncasecmp (name.UTF8Characters, "next", name.UTF8Length) == 0
+      	  || g_ascii_strncasecmp (name.UTF8Characters, "pause", name.UTF8Length) == 0
+      	  || g_ascii_strncasecmp (name.UTF8Characters, "play", name.UTF8Length) == 0
+      	  || g_ascii_strncasecmp (name.UTF8Characters, "previous", name.UTF8Length) == 0
+      	  || g_ascii_strncasecmp (name.UTF8Characters, "stop", name.UTF8Length) == 0)
       	  return BoolVariant (_result, true);
       return BoolVariant (_result, false);
 
diff --git a/browser-plugin/totemGMPPlayer.cpp b/browser-plugin/totemGMPPlayer.cpp
index f0cb252..c9ef165 100644
--- a/browser-plugin/totemGMPPlayer.cpp
+++ b/browser-plugin/totemGMPPlayer.cpp
@@ -243,8 +243,8 @@ totemGMPPlayer::SetPropertyByIndex (int aIndex,
 
     case eURL: {
       /* attribute AUTF8String URL; */
-      const char* url;
-      if (!GetStringFromArguments (aValue, 1, 0, url))
+      NPString url;
+      if (!GetNPStringFromArguments (aValue, 1, 0, url))
         return false;
 
       Plugin()->SetSrc (url); /* FIXMEchpe: use SetURL instead?? */
diff --git a/browser-plugin/totemNPNGlue.cpp b/browser-plugin/totemNPNGlue.cpp
index ab30d82..f55bd31 100644
--- a/browser-plugin/totemNPNGlue.cpp
+++ b/browser-plugin/totemNPNGlue.cpp
@@ -297,7 +297,28 @@ void* NPN_MemDup (const void* aMem, uint32 aLen)
   return memcpy (dup, aMem, aLen);
 }
 
-char* NPN_StrDup (const char* aString)
+char* NPN_StrDup (const char *aString)
 {
-  return (char*) NPN_MemDup (aString, strlen (aString) + 1);
+  if (!aString)
+    return NULL;
+
+  return NPN_StrnDup (aString, strlen (aString));
+}
+
+char* NPN_StrnDup (const char *aString, uint32 aLen)
+{
+  if (!aString)
+    return NULL;
+
+  if (aLen < 0)
+    aLen = strlen (aString);
+
+  char* dup = (char*) NPN_MemAlloc (aLen + 1);
+  if (!dup)
+    return NULL;
+
+  memcpy (dup, aString, aLen);
+  dup[aLen] = '\0';
+
+  return dup;
 }
diff --git a/browser-plugin/totemNPObject.cpp b/browser-plugin/totemNPObject.cpp
index cbf37b1..8e8d403 100644
--- a/browser-plugin/totemNPObject.cpp
+++ b/browser-plugin/totemNPObject.cpp
@@ -263,22 +263,21 @@ totemNPObject::GetDoubleFromArguments (const NPVariant* argv,
 }
 
 bool
-totemNPObject::GetStringFromArguments (const NPVariant* argv,
-                                       uint32_t argc,
-                                       uint32_t argNum,
-                                       const char*& _result)
+totemNPObject::GetNPStringFromArguments (const NPVariant* argv,
+                                         uint32_t argc,
+                                         uint32_t argNum,
+                                         NPString& _result)
 {
   if (!CheckArg (argv, argc, argNum, NPVariantType_String))
     return false;
 
   NPVariant arg = argv[argNum];
   if (NPVARIANT_IS_STRING (arg)) {
-    NPString string = NPVARIANT_TO_STRING (arg);
-    // FIXMEchpe this assumes it's 0-terminated, check that this holds!
-    _result = string.UTF8Characters;
+    _result = NPVARIANT_TO_STRING (arg);
   } else if (NPVARIANT_IS_NULL (arg) ||
              NPVARIANT_IS_VOID (arg)) {
-    _result = NULL;
+    _result.UTF8Characters = NULL;
+    _result.UTF8Length = 0;
   }
 
   return true;
@@ -293,12 +292,11 @@ totemNPObject::DupStringFromArguments (const NPVariant* argv,
   NPN_MemFree (_result);
   _result = NULL;
 
-  const char *newValue;
-  if (!GetStringFromArguments (argv, argc, argNum, newValue))
+  NPString newValue;
+  if (!GetNPStringFromArguments (argv, argc, argNum, newValue))
     return false;
 
-  /* This assumes every NPString is 0-terminated. FIXMEchpe check that this holds! */
-  _result = NPN_StrDup (newValue);
+  _result = NPN_StrnDup (newValue.UTF8Characters, newValue.UTF8Length);
   return true;
 }
 
diff --git a/browser-plugin/totemNPObject.h b/browser-plugin/totemNPObject.h
index bb10461..17d5828 100644
--- a/browser-plugin/totemNPObject.h
+++ b/browser-plugin/totemNPObject.h
@@ -81,7 +81,7 @@ class totemNPObject : public NPObject {
     bool GetBoolFromArguments (const NPVariant*, uint32_t, uint32_t, bool&);
     bool GetInt32FromArguments (const NPVariant*, uint32_t, uint32_t, int32_t&);
     bool GetDoubleFromArguments (const NPVariant*, uint32_t, uint32_t, double&);
-    bool GetStringFromArguments (const NPVariant*, uint32_t, uint32_t, const char*&);
+    bool GetNPStringFromArguments (const NPVariant*, uint32_t, uint32_t, NPString&);
     bool DupStringFromArguments (const NPVariant*, uint32_t, uint32_t, char*&);
     bool GetObjectFromArguments (const NPVariant*, uint32_t, uint32_t, NPObject*&);
 
diff --git a/browser-plugin/totemNarrowSpacePlugin.cpp b/browser-plugin/totemNarrowSpacePlugin.cpp
index ff9969f..8c2b299 100644
--- a/browser-plugin/totemNarrowSpacePlugin.cpp
+++ b/browser-plugin/totemNarrowSpacePlugin.cpp
@@ -287,8 +287,8 @@ totemNarrowSpacePlayer::InvokeByIndex (int aIndex,
       
     case eSetBgColor: {
       /* void SetBgColor (in ACString color); */
-      const char *color;
-      if (!GetStringFromArguments (argv, argc, 0, color))
+      NPString color;
+      if (!GetNPStringFromArguments (argv, argc, 0, color))
         return false;
 
       Plugin()->SetBackgroundColor (color);
@@ -358,8 +358,8 @@ totemNarrowSpacePlayer::InvokeByIndex (int aIndex,
 
     case eSetMatrix: {
       /* void SetMatrix (in ACString matrix); */
-      const char *matrix;
-      if (!GetStringFromArguments (argv, argc, 0, matrix))
+      NPString matrix;
+      if (!GetNPStringFromArguments (argv, argc, 0, matrix))
         return false;
 
       Plugin()->SetMatrix (matrix);
@@ -372,8 +372,8 @@ totemNarrowSpacePlayer::InvokeByIndex (int aIndex,
 
     case eSetMovieName: {
       /* void SetMovieName (in AUTF8String movieName); */
-      const char *name;
-      if (!GetStringFromArguments (argv, argc, 0, name))
+      NPString name;
+      if (!GetNPStringFromArguments (argv, argc, 0, name))
         return false;
 
       Plugin()->SetMovieName (name);
@@ -386,8 +386,8 @@ totemNarrowSpacePlayer::InvokeByIndex (int aIndex,
 
     case eSetRectangle: {
       /* void SetRectangle (in ACString rect); */
-      const char *rectangle;
-      if (!GetStringFromArguments (argv, argc, 0, rectangle))
+      NPString rectangle;
+      if (!GetNPStringFromArguments (argv, argc, 0, rectangle))
         return false;
 
       Plugin()->SetRectangle (rectangle);
diff --git a/browser-plugin/totemPlugin.cpp b/browser-plugin/totemPlugin.cpp
index ecefa05..7bb16cf 100644
--- a/browser-plugin/totemPlugin.cpp
+++ b/browser-plugin/totemPlugin.cpp
@@ -368,25 +368,28 @@ totemPlugin::ClearPlaylist ()
 }
 
 int32_t
-totemPlugin::AddItem (const char* aURI)
+totemPlugin::AddItem (const NPString& aURI)
 {
-        if (!aURI || !aURI[0])
+        if (!aURI.UTF8Characters || !aURI.UTF8Length)
                 return -1;
 
         /* FIXMEchpe: resolve against mBaseURI or mSrcURI ?? */
 
-	D ("AddItem '%s'", aURI);
-
 	/* FIXME: queue the action instead */
 	if (!mViewerReady)
 		return false;
 
 	assert (mViewerProxy);
-	dbus_g_proxy_call_no_reply (mViewerProxy,
+
+        char *uri = g_strndup (aURI.UTF8Characters, aURI.UTF8Length);
+	D ("AddItem '%s'", uri);
+
+        dbus_g_proxy_call_no_reply (mViewerProxy,
 				    "AddItem",
-				    G_TYPE_STRING, aURI,
+				    G_TYPE_STRING, uri,
 				    G_TYPE_INVALID,
 				    G_TYPE_INVALID);
+        g_free (uri);
 
 	return 0;
 }
@@ -418,31 +421,31 @@ totemPlugin::SetControllerVisible (bool enabled)
 }
 
 void
-totemPlugin::SetBackgroundColor (const char* color)
+totemPlugin::SetBackgroundColor (const NPString& color)
 {
   g_free (mBackgroundColor);
-  mBackgroundColor = g_strdup (color);
+  mBackgroundColor = g_strndup (color.UTF8Characters, color.UTF8Length);
 }
 
 void
-totemPlugin::SetMatrix (const char* matrix)
+totemPlugin::SetMatrix (const NPString& matrix)
 {
   g_free (mMatrix);
-  mMatrix = g_strdup (matrix);
+  mMatrix = g_strndup (matrix.UTF8Characters, matrix.UTF8Length);
 }
 
 void
-totemPlugin::SetRectangle (const char *rectangle)
+totemPlugin::SetRectangle (const NPString& rectangle)
 {
   g_free (mRectangle);
-  mRectangle = g_strdup (rectangle);
+  mRectangle = g_strndup (rectangle.UTF8Characters, rectangle.UTF8Length);
 }
 
 void
-totemPlugin::SetMovieName (const char *name)
+totemPlugin::SetMovieName (const NPString& name)
 {
   g_free (mMovieName);
-  mMovieName = g_strdup (name);
+  mMovieName = g_strndup (name.UTF8Characters, name.UTF8Length);
 }
 
 void
@@ -1517,6 +1520,30 @@ totemPlugin::SetSrc (const char* aURL)
         return true;
 }
 
+bool
+totemPlugin::SetSrc (const NPString& aURL)
+{
+        g_free (mSrcURI);
+
+	/* If |src| is empty, don't resolve the URI! Otherwise we may
+	 * try to load an (probably iframe) html document as our video stream.
+	 */
+	if (!aURL.UTF8Characters || !aURL.UTF8Length) {
+              mSrcURI = NULL;
+              return true;
+        }
+
+        mSrcURI = g_strndup (aURL.UTF8Characters, aURL.UTF8Length);
+
+        if (mAutoPlay) {
+                RequestStream (false);
+        } else {
+                mWaitingForButtonPress = true;
+        }
+
+        return true;
+}
+
 #ifdef TOTEM_GMP_PLUGIN
 
 void
diff --git a/browser-plugin/totemPlugin.h b/browser-plugin/totemPlugin.h
index c118249..fe7101f 100644
--- a/browser-plugin/totemPlugin.h
+++ b/browser-plugin/totemPlugin.h
@@ -339,11 +339,12 @@ class totemPlugin {
     NPObject* GetNPObject (ObjectEnum which);
 
     bool SetSrc (const char* aURL);
+    bool SetSrc (const NPString& aURL);
     const char* Src() const { return mSrcURI; }
 
     void Command (const char *aCommand);
     void ClearPlaylist ();
-    int32_t AddItem (const char*);
+    int32_t AddItem (const NPString&);
 
     void SetIsWindowless (bool enabled) { mIsWindowless = enabled; }
     bool IsWindowless () const { return mIsWindowless; }
@@ -378,16 +379,16 @@ class totemPlugin {
     void SetPlayEveryFrame (bool enabled) { mPlayEveryFrame = enabled; }
     bool PlayEveryFrame () const { return mPlayEveryFrame; }
 
-    void SetBackgroundColor (const char* color);
+    void SetBackgroundColor (const NPString& color);
     const char *BackgroundColor () const { return mBackgroundColor; }
 
-    void SetMatrix (const char* matrix);
+    void SetMatrix (const NPString& matrix);
     const char* Matrix () const { return mMatrix; }
 
-    void SetRectangle (const char *rectangle);
+    void SetRectangle (const NPString& rectangle);
     const char* Rectangle () const { return mRectangle; }
 
-    void SetMovieName (const char *name);
+    void SetMovieName (const NPString& name);
     const char* MovieName () const { return mMovieName; }
 
     void SetResetPropertiesOnReload (bool enabled) { mResetPropertiesOnReload = enabled; }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]