Bug 1073350: Validate that returned decoded Shmems have enough data r=cpearce
☠☠ backed out by 66ad106fcb68 ☠ ☠
authorRandell Jesup <rjesup@jesup.org>
Wed, 01 Oct 2014 20:39:08 -0400
changeset 208303 0ae42938a72d0ed92e44cdda74524fddf641ba9f
parent 208302 1f5252ebc2c0cad0f72c8761e5b06c1229d3bcfe
child 208304 812bc959f32bd936fd5c5d1f62cc883f114adad1
push id49891
push userrjesup@wgate.com
push dateThu, 02 Oct 2014 00:41:05 +0000
treeherdermozilla-inbound@0ae42938a72d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1073350
milestone35.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 1073350: Validate that returned decoded Shmems have enough data r=cpearce
content/media/gmp/GMPVideoDecoderParent.cpp
content/media/gmp/GMPVideoi420FrameImpl.cpp
content/media/gmp/GMPVideoi420FrameImpl.h
--- a/content/media/gmp/GMPVideoDecoderParent.cpp
+++ b/content/media/gmp/GMPVideoDecoderParent.cpp
@@ -223,16 +223,20 @@ GMPVideoDecoderParent::ActorDestroy(Acto
 
 bool
 GMPVideoDecoderParent::RecvDecoded(const GMPVideoi420FrameData& aDecodedFrame)
 {
   if (!mCallback) {
     return false;
   }
 
+  if (!CheckFrameData(aDecodedFrame)) {
+    LOGE(("%s: Decoded frame corrupt, ignoring", __FUNCTION__));
+    return false;
+  }
   auto f = new GMPVideoi420FrameImpl(aDecodedFrame, &mVideoHost);
 
   // Ignore any return code. It is OK for this to fail without killing the process.
   mCallback->Decoded(f);
 
   return true;
 }
 
--- a/content/media/gmp/GMPVideoi420FrameImpl.cpp
+++ b/content/media/gmp/GMPVideoi420FrameImpl.cpp
@@ -58,16 +58,41 @@ GMPVideoi420FrameImpl::GetFrameFormat()
 }
 
 void
 GMPVideoi420FrameImpl::Destroy()
 {
   delete this;
 }
 
+/* static */ bool
+CheckFrameData(const GMPVideoi420FrameData& aFrameData)
+{
+  // We may be passed the "wrong" shmem (one smaller than the actual size).
+  // This implies a bug or serious error on the child size.  Ignore this frame if so.
+  // Note: Size() greater than expected is also an error, but with no negative consequences
+  int32_t half_width = (aFrameData.mWidth() + 1) / 2;
+  if ((aFrameData.mYPlane().mStride() <= 0) || (aFrameData.mYPlane().mSize() <= 0) ||
+      (aFrameData.mUPlane().mStride() <= 0) || (aFrameData.mUPlane().mSize() <= 0) ||
+      (aFrameData.mVPlane().mStride() <= 0) || (aFrameData.mVPlane().mSize() <= 0) ||
+      (aFrameData.mYPlane().mSize() > (int32_t) aFrameData.mYPlane().mBuffer().Size<uint8_t>()) ||
+      (aFrameData.mUPlane().mSize() > (int32_t) aFrameData.mUPlane().mBuffer().Size<uint8_t>()) ||
+      (aFrameData.mVPlane().mSize() > (int32_t) aFrameData.mVPlane().mBuffer().Size<uint8_t>()) ||
+      (aFrameData.mYPlane().mStride() < aFrameData.mWidth()) ||
+      (aFrameData.mUPlane().mStride() < half_width) ||
+      (aFrameData.mVPlane().mStride() < half_width) ||
+      (aFrameData.mYPlane().mSize() < aFrameData.mYPlane().mStride() * aFrameData.mHeight()) ||
+      (aFrameData.mUPlane().mSize() < aFrameData.mUPlane().mStride() * ((aFrameData.mHeight()+1)/2)) ||
+      (aFrameData.mVPlane().mSize() < aFrameData.mVPlane().mStride() * ((aFrameData.mHeight()+1)/2)))
+  {
+    return false;
+  }
+  return true;
+}
+
 bool
 GMPVideoi420FrameImpl::CheckDimensions(int32_t aWidth, int32_t aHeight,
                                        int32_t aStride_y, int32_t aStride_u, int32_t aStride_v)
 {
   int32_t half_width = (aWidth + 1) / 2;
   if (aWidth < 1 || aHeight < 1 || aStride_y < aWidth ||
                                    aStride_u < half_width ||
                                    aStride_v < half_width) {
--- a/content/media/gmp/GMPVideoi420FrameImpl.h
+++ b/content/media/gmp/GMPVideoi420FrameImpl.h
@@ -11,16 +11,19 @@
 #include "mozilla/ipc/Shmem.h"
 #include "GMPVideoPlaneImpl.h"
 
 namespace mozilla {
 namespace gmp {
 
 class GMPVideoi420FrameData;
 
+static bool
+CheckFrameData(const GMPVideoi420FrameData& aFrameData);
+
 class GMPVideoi420FrameImpl : public GMPVideoi420Frame
 {
   friend struct IPC::ParamTraits<mozilla::gmp::GMPVideoi420FrameImpl>;
 public:
   explicit GMPVideoi420FrameImpl(GMPVideoHostImpl* aHost);
   GMPVideoi420FrameImpl(const GMPVideoi420FrameData& aFrameData, GMPVideoHostImpl* aHost);
   virtual ~GMPVideoi420FrameImpl();