Add rgb_frame size tracking and resizing to fix security issue with changing sizes.

Added negative checks on signed heights and widths, added negative check for signed heights and widths in backing store for video layering.

Patch by cdn@chromium.org:
http://codereview.chromium.org/2449006/show

BUG=45267
TEST=Run on linux with --enable-video-layering and use <video> tag

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49131 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 44 insertions, 17 deletions

diff --git a/chrome/browser/renderer_host/backing_store_x.cc b/chrome/browser/renderer_host/backing_store_x.cc
index 14d25a8..b7f5408 100644
--- a/chrome/browser/renderer_host/backing_store_x.cc
+++ b/chrome/browser/renderer_host/backing_store_x.cc
@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <utility>
+#include <limits>
 
 #include "app/surface/transport_dib.h"
 #include "app/x11_util.h"
@@ -29,6 +30,14 @@
 #include "skia/ext/platform_canvas.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
+// Assume that somewhere along the line, someone will do width * height * 4
+// with signed numbers. If the maximum value is 2**31, then 2**31 / 4 =
+// 2**29 and floor(sqrt(2**29)) = 23170.
+
+// Max height and width for layers
+static const int kMaxVideoLayerSize = 23170;
+
+
 // X Backing Stores:
 //
 // Unlike Windows, where the backing store is kept in heap memory, we keep our
@@ -160,10 +169,9 @@ void BackingStoreX::PaintToBackingStore(
 
   const int width = bitmap_rect.width();
   const int height = bitmap_rect.height();
-  // Assume that somewhere along the line, someone will do width * height * 4
-  // with signed numbers. If the maximum value is 2**31, then 2**31 / 4 =
-  // 2**29 and floor(sqrt(2**29)) = 23170.
-  if (width > 23170 || height > 23170)
+
+  if (width <= 0 || width > kMaxVideoLayerSize ||
+      height <= 0 || height > kMaxVideoLayerSize)
     return;
 
   TransportDIB* dib = process->GetTransportDIB(bitmap);
@@ -313,8 +321,16 @@ bool BackingStoreX::CopyFromBackingStore(const gfx::Rect& rect,
     memset(&shminfo, 0, sizeof(shminfo));
     image = XShmCreateImage(display_, visual, 32,
                             ZPixmap, NULL, &shminfo, width, height);
-
+    if (!image) {
+      return false;
+    }
     // Create the shared memory segment for the image and map it.
+    if (image->bytes_per_line == 0 || image->height == 0 ||
+        (std::numeric_limits<size_t>::max() / image->bytes_per_line) >
+        static_cast<size_t>(image->height)) {
+      XDestroyImage(image);
+      return false;
+    }
     shminfo.shmid = shmget(IPC_PRIVATE, image->bytes_per_line * image->height,
                            IPC_CREAT|0666);
     if (shminfo.shmid == -1) {
diff --git a/chrome/browser/renderer_host/video_layer_x.cc b/chrome/browser/renderer_host/video_layer_x.cc
index c204b95..09891b3 100644
--- a/chrome/browser/renderer_host/video_layer_x.cc
+++ b/chrome/browser/renderer_host/video_layer_x.cc
@@ -8,6 +8,14 @@
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "media/base/yuv_convert.h"
 
+
+// Assume that somewhere along the line, someone will do width * height * 4
+// with signed numbers. If the maximum value is 2**31, then 2**31 / 4 =
+// 2**29 and floor(sqrt(2**29)) = 23170.
+
+// Max height and width for layers
+static const int kMaxVideoLayerSize = 23170;
+
 VideoLayerX::VideoLayerX(RenderWidgetHost* widget,
                          const gfx::Size& size,
                          void* visual,
@@ -15,7 +23,8 @@ VideoLayerX::VideoLayerX(RenderWidgetHost* widget,
     : VideoLayer(widget, size),
       visual_(visual),
       depth_(depth),
-      display_(x11_util::GetXDisplay()) {
+      display_(x11_util::GetXDisplay()),
+      rgb_frame_size_(0) {
   DCHECK(!size.IsEmpty());
 
   // Create our pixmap + GC representing an RGB version of a video frame.
@@ -51,22 +60,22 @@ void VideoLayerX::CopyTransportDIB(RenderProcessHost* process,
   // Save location and size of destination bitmap.
   rgb_rect_ = bitmap_rect;
 
+  const int width = bitmap_rect.width();
+  const int height = bitmap_rect.height();
+  const size_t new_rgb_frame_size = static_cast<size_t>(width * height * 4);
+
+  if (width <= 0 || width > kMaxVideoLayerSize ||
+      height <= 0 || height > kMaxVideoLayerSize)
+    return;
+
   // Lazy allocate |rgb_frame_|.
-  if (!rgb_frame_.get()) {
+  if (!rgb_frame_.get() || rgb_frame_size_ < new_rgb_frame_size) {
     // TODO(scherkus): handle changing dimensions and re-allocating.
     CHECK(size() == rgb_rect_.size());
-
-    rgb_frame_.reset(new uint8[rgb_rect_.width() * rgb_rect_.height() * 4]);
+    rgb_frame_.reset(new uint8[new_rgb_frame_size]);
+    rgb_frame_size_ = new_rgb_frame_size;
   }
 
-  const int width = bitmap_rect.width();
-  const int height = bitmap_rect.height();
-  // Assume that somewhere along the line, someone will do width * height * 4
-  // with signed numbers. If the maximum value is 2**31, then 2**31 / 4 =
-  // 2**29 and floor(sqrt(2**29)) = 23170.
-  if (width > 23170 || height > 23170)
-    return;
-
   TransportDIB* dib = process->GetTransportDIB(bitmap);
   if (!dib)
     return;
diff --git a/chrome/browser/renderer_host/video_layer_x.h b/chrome/browser/renderer_host/video_layer_x.h
index a3c51b3..24bf316 100644
--- a/chrome/browser/renderer_host/video_layer_x.h
+++ b/chrome/browser/renderer_host/video_layer_x.h
@@ -42,6 +42,8 @@ class VideoLayerX : public VideoLayer {
 
   // Most recently converted frame stored as 32-bit ARGB.
   scoped_array<uint8> rgb_frame_;
+  size_t rgb_frame_size_;
+
   // Destination size and absolution position of the converted frame.
   gfx::Rect rgb_rect_;