From 7d60fb28dd7f2b2f46b2f5cd55f721245965faf1 Mon Sep 17 00:00:00 2001
From: "scherkus@chromium.org"
 <scherkus@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 8 Jun 2010 00:56:37 +0000
Subject: 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
---
 chrome/browser/renderer_host/backing_store_x.cc | 26 +++++++++++++++----
 chrome/browser/renderer_host/video_layer_x.cc   | 33 ++++++++++++++++---------
 chrome/browser/renderer_host/video_layer_x.h    |  2 ++
 3 files changed, 44 insertions(+), 17 deletions(-)

(limited to 'chrome')

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_;
 
-- 
cgit v1.1