Prevent calling internal metrics code with invalid values.

Currently we can call the internal metrics code in Chrome from the metrics extension with completely arbitrary values; some of which can cause the creation of invalid histograms which in turn can Chrome to crash. This CL sanitizes the inputs to the extension so that the histogram constructed is valid.

Currently this is causing one crash due to code from http://codereview.chromium.org/8819013
This CL fixes the issue with the extension and fixes the crash. The issues with the JS code still needs to be fixed in another CL.

R=jar@chromium.org
BUG=chromium:24115
TEST=Tried the repro case for the crash several times to confirm that it isn't happening anymore.

Review URL: http://codereview.chromium.org/9113002

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

2 files changed, 19 insertions, 4 deletions

diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index a44a2eb..a6f2b27 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -94,6 +94,10 @@ Histogram* Histogram::FactoryGet(const std::string& name,
   if (maximum > kSampleType_MAX - 1)
     maximum = kSampleType_MAX - 1;
 
+  DCHECK_GT(maximum, minimum);
+  DCHECK_GT((Sample) bucket_count, 2);
+  DCHECK_LE((Sample) bucket_count, maximum - minimum + 2);
+
   if (!StatisticsRecorder::FindHistogram(name, &histogram)) {
     // Extra variable is not needed... but this keeps this section basically
     // identical to other derived classes in this file (and compiler will
@@ -816,6 +820,10 @@ Histogram* LinearHistogram::FactoryGet(const std::string& name,
   if (maximum > kSampleType_MAX - 1)
     maximum = kSampleType_MAX - 1;
 
+  DCHECK_GT(maximum, minimum);
+  DCHECK_GT((Sample) bucket_count, 2);
+  DCHECK_LE((Sample) bucket_count, maximum - minimum + 2);
+
   if (!StatisticsRecorder::FindHistogram(name, &histogram)) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     LinearHistogram* tentative_histogram =
diff --git a/base/metrics/histogram.h b/base/metrics/histogram.h
index 0b94613..d203e54 100644
--- a/base/metrics/histogram.h
+++ b/base/metrics/histogram.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -430,8 +430,15 @@ class BASE_EXPORT Histogram {
   };
 
   //----------------------------------------------------------------------------
-  // minimum should start from 1. 0 is invalid as a minimum. 0 is an implicit
-  // default underflow bucket.
+  // For a valid histogram, input should follow these restrictions:
+  // minimum > 0 (if a minimum below 1 is specified, it will implicitly be
+  //              normalized up to 1)
+  // maximum > minimum
+  // buckets > 2 [minimum buckets needed: underflow, overflow and the range]
+  // Additionally,
+  // buckets <= (maximum - minimum + 2) - this is to ensure that we don't have
+  // more buckets than the range of numbers; having more buckets than 1 per
+  // value in the range would be nonsensical.
   static Histogram* FactoryGet(const std::string& name,
                                Sample minimum,
                                Sample maximum,