Title: [93749] trunk/Source/WebCore
Revision
93749
Author
commit-qu...@webkit.org
Date
2011-08-24 17:35:44 -0700 (Wed, 24 Aug 2011)

Log Message

Resolve potential integer overflow in memory allocation, and ensure
that allocation succeeds.

Patch by Chris Palmer <pal...@google.com> on 2011-08-24
Reviewed by Kenneth Russell.

* platform/audio/AudioArray.h:
(WebCore::AudioArray::allocate): Check for integer overflow.
(WebCore::AudioArray::at): Document the safety assertion.
(WebCore::AudioArray::zero): Document the safety assertion.
(WebCore::AudioArray::zeroRange): Document the safety assertion.
(WebCore::AudioArray::copyToRange): Document the safety assertion.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (93748 => 93749)


--- trunk/Source/WebCore/ChangeLog	2011-08-24 23:04:12 UTC (rev 93748)
+++ trunk/Source/WebCore/ChangeLog	2011-08-25 00:35:44 UTC (rev 93749)
@@ -1,3 +1,17 @@
+2011-08-24  Chris Palmer  <pal...@google.com>
+
+        Resolve potential integer overflow in memory allocation, and ensure
+        that allocation succeeds.
+
+        Reviewed by Kenneth Russell.
+
+        * platform/audio/AudioArray.h:
+        (WebCore::AudioArray::allocate): Check for integer overflow.
+        (WebCore::AudioArray::at): Document the safety assertion.
+        (WebCore::AudioArray::zero): Document the safety assertion.
+        (WebCore::AudioArray::zeroRange): Document the safety assertion.
+        (WebCore::AudioArray::copyToRange): Document the safety assertion.
+
 2011-08-24  Raphael Kubo da Costa  <k...@profusion.mobi>
 
         [EFL] Fix build with ENABLE_GEOLOCATION.

Modified: trunk/Source/WebCore/platform/audio/AudioArray.h (93748 => 93749)


--- trunk/Source/WebCore/platform/audio/AudioArray.h	2011-08-24 23:04:12 UTC (rev 93748)
+++ trunk/Source/WebCore/platform/audio/AudioArray.h	2011-08-25 00:35:44 UTC (rev 93749)
@@ -53,6 +53,13 @@
     // if re-allocated. Allocations are zero-initialized.
     void allocate(size_t n)
     {
+        // Although n is a size_t, its true limit is max unsigned because we use unsigned in zeroRange()
+        // and copyToRange(). Also check for integer overflow.
+        if (n > std::numeric_limits<unsigned>::max() / sizeof(T))
+            CRASH();
+      
+        unsigned initialSize = sizeof(T) * n;
+
         // 16-byte alignment for 128bit SIMD.
         const size_t alignment = 16;
 
@@ -66,7 +73,13 @@
             // then we'll have to reallocate and from then on allocate extra.
             static size_t extraAllocationBytes = 0;
 
-            T* allocation = static_cast<T*>(fastMalloc(sizeof(T) * n + extraAllocationBytes));
+            // Again, check for integer overflow.
+            if (initialSize + extraAllocationBytes < initialSize)
+                CRASH();
+
+            T* allocation = static_cast<T*>(fastMalloc(initialSize + extraAllocationBytes));
+            if (!allocation)
+                CRASH();
             T* alignedData = alignedAddress(allocation, alignment);
 
             if (alignedData == allocation || extraAllocationBytes == alignment) {
@@ -88,13 +101,19 @@
 
     T& at(size_t i)
     {
+        // Note that although it is a size_t, m_size is now guaranteed to be
+        // no greater than max unsigned. This guarantee is enforced in allocate().
         ASSERT(i < size());
         return data()[i];
     }
 
     T& operator[](size_t i) { return at(i); }
 
-    void zero() { memset(this->data(), 0, sizeof(T) * this->size()); }
+    void zero()
+    {
+        // This multiplication is made safe by the check in allocate().
+        memset(this->data(), 0, sizeof(T) * this->size());
+    }
 
     void zeroRange(unsigned start, unsigned end)
     {
@@ -103,6 +122,8 @@
         if (!isSafe)
             return;
 
+        // This _expression_ cannot overflow because end - start cannot be
+        // greater than m_size, which is safe due to the check in allocate().
         memset(this->data() + start, 0, sizeof(T) * (end - start));
     }
 
@@ -113,6 +134,8 @@
         if (!isSafe)
             return;
 
+        // This _expression_ cannot overflow because end - start cannot be
+        // greater than m_size, which is safe due to the check in allocate().
         memcpy(this->data() + start, sourceData, sizeof(T) * (end - start));
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to