CFArray: fix heap overflows in mutable-array growth and copy

ChangeLog

@@ -1,3 +1,13 @@
+2026-06-28  Todd White  <todd.white@thalion.global>
+	* Source/CFArray.c (CFArrayCheckCapacityAndGrow): Grow to at least
+	the requested capacity, not just by DEFAULT_ARRAY_CAPACITY.
+	(CFArrayReplaceValues): Capture the original count up front and
+	recompute the destination pointer after growing, fixing a too-small
+	grow, an over-long memmove, and a write into the pre-grow buffer.
+	(CFArrayCreateMutableCopy): Clamp the capacity to the source count
+	on the non-Objective-C path before writing the contents.
+	* Tests/CFArray/mutablearray.m: Regression tests.
+
 2021-09-30  Frederik Seiffert <frederik@algoriddim.com>
 	* Source/CFDate.c: Fix logic in CFGregorianDateIsValid()
 

Source/CFArray.c

@@ -446,7 +446,10 @@ CFArrayCheckCapacityAndGrow (CFMutableArrayRef array, CFIndex newCapacity)
 
   if (mArray->_capacity < newCapacity)
     {
-      newCapacity = mArray->_capacity + DEFAULT_ARRAY_CAPACITY;
+      /* Grow by at least DEFAULT_ARRAY_CAPACITY, but never to less than the
+         requested capacity (a single operation may need more than that). */
+      if (newCapacity < mArray->_capacity + DEFAULT_ARRAY_CAPACITY)
+        newCapacity = mArray->_capacity + DEFAULT_ARRAY_CAPACITY;
 
       mArray->_contents = CFAllocatorReallocate (CFGetAllocator (mArray),
                                                  mArray->_contents,
@@ -520,6 +523,7 @@ CFArrayCreateMutableCopy (CFAllocatorRef allocator, CFIndex capacity,
 
   CFMutableArrayRef new;
   const CFArrayCallBacks *callbacks;
+  CFIndex count;
 
   if (!array)
     return NULL;
@@ -529,13 +533,18 @@ CFArrayCreateMutableCopy (CFAllocatorRef allocator, CFIndex capacity,
   else
     callbacks = array->_callBacks;
 
+  /* The contents are written directly below, so the array must be large
+     enough to hold them regardless of the requested capacity. */
+  count = CFArrayGetCount (array);
+  if (count > capacity)
+    capacity = count;
+
   new = CFArrayCreateMutable (allocator, capacity, callbacks);
   if (new)
     {
       CFIndex idx;
-      CFIndex count;
 
-      for (idx = 0, count = CFArrayGetCount (array); idx < count; ++idx)
+      for (idx = 0; idx < count; ++idx)
         {
           new->_contents[idx] = callbacks->retain
             ? callbacks->retain (NULL, CFArrayGetValueAtIndex (array, idx))
@@ -638,12 +647,12 @@ CFArrayReplaceValues (CFMutableArrayRef array, CFRange range,
     }
 
   const void **start;
-  const void **end;
   CFAllocatorRef alloc;
+  CFIndex oldCount;
 
-  start = array->_contents + range.location;
-  end = start + range.length;
   alloc = CFGetAllocator (array);
+  oldCount = array->_count;
+  start = array->_contents + range.location;
 
   /* Release values if needed */
   if (range.length > 0)
@@ -652,31 +661,31 @@ CFArrayReplaceValues (CFMutableArrayRef array, CFRange range,
       if (release)
         {
           const void **current = start;
+          const void **end = start + range.length;
           while (current < end)
             release (alloc, *(current++));
         }
-      array->_count -= range.length;
     }
 
-  /* Move remaining values if required */
+  /* Move the values after the replaced range into their new position. */
   if (range.length != newCount)
     {
-      CFIndex newSize;
+      CFIndex tail = oldCount - (range.location + range.length);
 
-      newSize = array->_count - range.length + newCount;
-      CFArrayCheckCapacityAndGrow (array, newSize);
+      CFArrayCheckCapacityAndGrow (array, oldCount - range.length + newCount);
+      /* The grow above may have reallocated _contents, so recompute. */
+      start = array->_contents + range.location;
 
-      memmove (start + newCount, end,
-               (array->_count - range.location +
-                range.length) * sizeof (void *));
+      memmove (start + newCount, start + range.length,
+               tail * sizeof (void *));
     }
 
   /* Insert new values */
   if (newCount > 0)
     {
       CFArrayRetainCallBack retain = array->_callBacks->retain;
       const void **current = start;
-      end = current + newCount; /* New end... */
+      const void **end = current + newCount;
       if (retain)
         {
           while (current < end)
@@ -687,8 +696,9 @@ CFArrayReplaceValues (CFMutableArrayRef array, CFRange range,
           while (current < end)
             *(current++) = *(newValues++);
         }
-      array->_count += newCount;
     }
+
+  array->_count = oldCount - range.length + newCount;
 }
 
 void

Tests/CFArray/mutablearray.m

@@ -41,7 +41,36 @@ int main (void)
 
   n = CFArrayBSearchValues (ma, CFRangeMake(0, len), (const void*)6, comp, NULL);
   PASS_CF(n == 5, "Index of value between values is %d.", (int)n);
-
+
+  {
+    const void *big[20];
+    CFArrayRef bigArr;
+    CFMutableArrayRef mc;
+    CFMutableArrayRef target;
+    int i;
+
+    for (i = 0; i < 20; i++)
+      big[i] = (const void *)(CFIndex)(i + 1);
+    bigArr = CFArrayCreate (NULL, big, 20, NULL);
+
+    /* A mutable copy whose requested capacity is smaller than the source
+       count must still hold every value (no write past the buffer). */
+    mc = CFArrayCreateMutableCopy (NULL, 0, bigArr);
+    PASS_CF(CFArrayGetCount (mc) == 20
+      && CFArrayGetValueAtIndex (mc, 19) == (const void *)(CFIndex)20,
+      "MutableCopy with capacity 0 holds all 20 values.");
+    CFRelease (mc);
+
+    /* Appending a whole array grows by more than the default chunk at once. */
+    target = CFArrayCreateMutable (NULL, 0, NULL);
+    CFArrayAppendArray (target, bigArr, CFRangeMake (0, 20));
+    PASS_CF(CFArrayGetCount (target) == 20
+      && CFArrayGetValueAtIndex (target, 19) == (const void *)(CFIndex)20,
+      "AppendArray of 20 values grows the array correctly.");
+    CFRelease (target);
+    CFRelease (bigArr);
+  }
+
   return 0;
 }