CFAttributedString: fix allocation, use-after-free and OOB bugs

ChangeLog

@@ -1,3 +1,18 @@
+2026-06-28  Todd White  <todd.white@thalion.global>
+	* Source/CFAttributedString.c (CFATTRIBUTESTRING_SIZE,
+	CFMUTABLEATTRIBUTESTRING_SIZE): Compute the instance extra size as
+	sizeof(struct ...) - sizeof(CFRuntimeBase).
+	(CFAttributedStringGetBlankAttribute): Keep a reference to the cached
+	copy instead of dereferencing the released original.
+	(InsertAttributesAtIndex, RemoveAttributesAtIndex): Reallocate by
+	byte count and update _attribCap when growing/shrinking.
+	(CFAttributedStringCoalesce): Do not read element [-1] when the range
+	starts at index 0.
+	* Source/GSHashTable.c, Source/GSHashTable.h
+	(GSHashTableAddValueCounted): New counted add.
+	* Source/CFBag.c (CFBagAddValue): Use it so a bag tracks value counts.
+	* Tests/CFAttributedString/mutable.m: Test growing the attribute array.
+
 2021-09-30  Frederik Seiffert <frederik@algoriddim.com>
 	* Source/CFDate.c: Fix logic in CFGregorianDateIsValid()
 

Source/CFAttributedString.c

@@ -119,13 +119,19 @@ CFAttributedStringCacheAttribute (CFDictionaryRef attribs)
   if (cachedAttr == NULL)
     {
       CFDictionaryRef insert;
-      
+
       insert = CFDictionaryCreateCopy (NULL, attribs);
       CFBagAddValue (_kCFAttributedStringCache, insert);
       cachedAttr = insert;
       CFRelease (insert);
     }
-
+  else
+    {
+      /* The cache is a counted set: record this additional reference so the
+         attribute is only freed once every user has uncached it. */
+      CFBagAddValue (_kCFAttributedStringCache, cachedAttr);
+    }
+
   GSMutexUnlock (&_kCFAttributedStringCacheLock);
 
   return cachedAttr;
@@ -149,13 +155,17 @@ CFAttributedStringGetBlankAttribute (void)
       GSMutexLock (&_kCFAttributedStringBlankAttributeLock);
       if (_kCFAttributedStringBlankAttribute == NULL)
         {
-          _kCFAttributedStringBlankAttribute = CFDictionaryCreate (NULL,
-            NULL, NULL, 0,
+          CFDictionaryRef blank;
+
+          blank = CFDictionaryCreate (NULL, NULL, NULL, 0,
             &kCFCopyStringDictionaryKeyCallBacks,
             &kCFTypeDictionaryValueCallBacks);
-          /* Cache the blank attribute in case anyone wants to use it. */
-          CFAttributedStringCacheAttribute (_kCFAttributedStringBlankAttribute);
-          CFRelease (_kCFAttributedStringBlankAttribute);
+          /* CFAttributedStringCacheAttribute() caches and returns a *copy*;
+             keep a reference to that cached copy (not the temporary we just
+             created, which we then release). */
+          _kCFAttributedStringBlankAttribute =
+            CFRetain (CFAttributedStringCacheAttribute (blank));
+          CFRelease (blank);
         }
       GSMutexUnlock (&_kCFAttributedStringBlankAttributeLock);
     }
@@ -284,8 +294,8 @@ CFAttributedStringGetTypeID (void)
   return _kCFAttributedStringTypeID;
 }
 
-#define CFATTRIBUTESTRING_SIZE sizeof(CFRuntimeClass) \
-  - sizeof(struct __CFAttributedString)
+#define CFATTRIBUTESTRING_SIZE (sizeof(struct __CFAttributedString) \
+  - sizeof(CFRuntimeBase))
 
 static CFAttributedStringRef
 CFAttributedStringCreateInlined (CFAllocatorRef alloc, CFStringRef str,
@@ -451,9 +461,11 @@ InsertAttributesAtIndex (CFMutableAttributedStringRef str, CFIndex idx,
   if (working->_attribCount == working->_attribCap)
     {
       /* Grow */
+      working->_attribCap <<= 1;
       working->_attribs = CFAllocatorReallocate (alloc,
                                                  working->_attribs,
-                                                 (working->_attribCap << 1),
+                                                 working->_attribCap
+                                                 * sizeof (Attr),
                                                  0);
     }
 
@@ -537,9 +549,11 @@ RemoveAttributesAtIndex (CFMutableAttributedStringRef str, CFRange range)
           && working->_attribCount > 9)
         {
           /* Shrink */
+          working->_attribCap >>= 1;
           working->_attribs = CFAllocatorReallocate (alloc,
                                                      working->_attribs,
-                                                     (working->_attribCap >> 1),
+                                                     working->_attribCap
+                                                     * sizeof (Attr),
                                                      0);
         }
     }
@@ -568,9 +582,11 @@ CFAttributedStringCoalesce (CFMutableAttributedStringRef str, CFRange range)
             }
         }
 
-      cur = range.location;
+      /* Each step compares with the preceding entry, so never start at
+         index 0 (there is nothing before it to coalesce with). */
+      cur = range.location > 0 ? range.location : 1;
       end = range.location + range.length;
-      
+
       while (cur < end)
         {
           if (array[cur - 1].attrib == array[cur].attrib)
@@ -583,8 +599,8 @@ CFAttributedStringCoalesce (CFMutableAttributedStringRef str, CFRange range)
     }
 }
 
-#define CFMUTABLEATTRIBUTESTRING_SIZE sizeof(CFRuntimeClass) \
-  - sizeof(struct __CFMutableAttributedString)
+#define CFMUTABLEATTRIBUTESTRING_SIZE (sizeof(struct __CFMutableAttributedString) \
+  - sizeof(CFRuntimeBase))
 
 CFMutableAttributedStringRef
 CFAttributedStringCreateMutable (CFAllocatorRef alloc, CFIndex maxLength)

Source/CFBag.c

@@ -198,7 +198,9 @@ CFBagCreateMutableCopy (CFAllocatorRef allocator, CFIndex capacity,
 void
 CFBagAddValue (CFMutableBagRef bag, const void *value)
 {
-  GSHashTableAddValue ((GSHashTableRef)bag, value, value);
+  /* A bag is a counted collection: adding a value already present must
+     increase its count (CFBagRemoveValue decreases it). */
+  GSHashTableAddValueCounted ((GSHashTableRef)bag, value, value);
 }
 
 void

Source/GSHashTable.c

@@ -636,6 +636,30 @@ GSHashTableAddValue (GSHashTableRef table, const void *key, const void *value)
     }
 }
 
+void
+GSHashTableAddValueCounted (GSHashTableRef table, const void *key,
+                            const void *value)
+{
+  GSHashTableBucket *bucket;
+
+  GSHashTableGrowIfNeeded (table);
+
+  bucket = GSHashTableFindBucket (table, key, _kGSHashTableRetrieve);
+  if (!bucket)
+    bucket = GSHashTableFindBucket (table, key, _kGSHashTableInsert);
+
+  if (bucket->count <= 0)
+    {
+      GSHashTableAddKeyValuePair (table, bucket, key, value);
+      table->_count += 1;
+    }
+  else
+    {
+      /* Already present: record one more reference. */
+      bucket->count += 1;
+    }
+}
+
 void
 GSHashTableReplaceValue (GSHashTableRef table, const void *key,
                          const void *value)

Source/GSHashTable.h

@@ -132,6 +132,12 @@ GSHashTableCreateMutableCopy (CFAllocatorRef alloc, GSHashTableRef table,
 GS_PRIVATE void
 GSHashTableAddValue (GSHashTableRef table, const void *key, const void *value);
 
+/* Like GSHashTableAddValue but, for a value that is already present,
+   increments its count (for counted collections such as CFBag). */
+GS_PRIVATE void
+GSHashTableAddValueCounted (GSHashTableRef table, const void *key,
+                            const void *value);
+
 GS_PRIVATE void
 GSHashTableReplaceValue (GSHashTableRef table, const void *key,
                          const void *value);

Tests/CFAttributedString/mutable.m

@@ -1,4 +1,6 @@
 #include "CoreFoundation/CFAttributedString.h"
+#include "CoreFoundation/CFString.h"
+#include "CoreFoundation/CFDictionary.h"
 #include "../CFTesting.h"
 
 int main (void)
@@ -67,7 +69,35 @@ int main (void)
   CFRelease (attrib3);
   CFRelease (mstr);
   CFRelease (str);
-
+
+  {
+    /* Setting an attribute at many distinct positions grows the internal
+       attribute array past its initial capacity of 8. */
+    CFMutableAttributedStringRef big;
+    CFDictionaryRef at;
+    CFIndex i;
+    Boolean ok;
+
+    big = CFAttributedStringCreateMutable (NULL, 0);
+    CFAttributedStringReplaceString (big, CFRangeMake (0, 0),
+                                     CFSTR ("0123456789abcdefghij"));
+    for (i = 0; i < 20; i++)
+      {
+        CFStringRef key = CFStringCreateWithFormat (NULL, NULL, CFSTR ("k%d"),
+                                                    (int) i);
+        CFAttributedStringSetAttribute (big, CFRangeMake (i, 1), key,
+                                        CFSTR ("v"));
+        CFRelease (key);
+      }
+    at = CFAttributedStringGetAttributes (big, 5, &r);
+    ok = CFAttributedStringGetLength (big) == 20
+      && at != NULL
+      && CFDictionaryGetValue (at, CFSTR ("k5")) != NULL;
+    PASS_CF (ok,
+      "Growing the attribute array past its initial capacity works.");
+    CFRelease (big);
+  }
+
   return 0;
 }