Added a few range checks in incomplete code and changed stack usage to reduce

chances of stack overflow in dictionary operations on very large dictionaries.


git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@18850 72102866-910b-0410-8b05-ffd578937521

ChangeLog

@@ -1,8 +1,14 @@
+2004-03-16 Richard Frith-Macdonald <rfm@gnu.org>
+
+	* Source/NSDictionary.m: Don't use stack as temporary storage for
+	large numbers of objects ... reduce chances of stack overflow.
+	* Source/NSIndexSet.m: Added a few range checks.
+
 2004-03-16  Willem Rein Oudshoorn  <Wim.Oudshoorn@agilisys.com>
 
 	* Source/NSNotificationCenter.m ([NSNotificationCenter -removeObserver:name:object:]): 
-	Just return if all arguments are nil instead of throwing exception.  The new
+	Just return if all arguments are nil instead of throwing exception.
-	behaviour is compatible with MacOSX.
+	The new behaviour is compatible with MacOSX.
 
 2004-03-15 Richard Frith-Macdonald <rfm@gnu.org>
 

Source/NSDictionary.m

@@ -351,17 +351,24 @@ static SEL	appSel;
 - (id) initWithObjects: (NSArray*)objects forKeys: (NSArray*)keys
 {
   unsigned	objectCount = [objects count];
-  id		os[objectCount];
-  id		ks[objectCount];
   
   if (objectCount != [keys count])
     {
       [NSException raise: NSInvalidArgumentException
 		  format: @"init with obj and key arrays of different sizes"];
     }
-  [objects getObjects: os];
+  else
-  [keys getObjects: ks];
+    {
-  return [self initWithObjects: os forKeys: ks count: objectCount];
+      GS_BEGINIDBUF(o, objectCount*2)
+
+      [objects getObjects: o];
+      [keys getObjects: o + objectCount];
+      self = [self initWithObjects: o
+			   forKeys: o + objectCount
+			     count: objectCount];
+      GS_ENDIDBUF()
+    }
+  return self;
 }
 
 /**
@@ -429,13 +436,12 @@ static SEL	appSel;
 
   if (c > 0)
     {
-      id		os[c];
-      id		ks[c];
       id		k;
       NSEnumerator	*e = [other keyEnumerator];
       unsigned		i = 0;
       IMP		nxtObj = [e methodForSelector: nxtSel];
       IMP		otherObj = [other methodForSelector: objSel];
+      GS_BEGINIDBUF(o, c*2)
 
       if (shouldCopy)
 	{
@@ -443,15 +449,15 @@ static SEL	appSel;
 
 	  while ((k = (*nxtObj)(e, nxtSel)) != nil)
 	    {
-	      ks[i] = k;
+	      o[i] = k;
-	      os[i] = [(*otherObj)(other, objSel, k) copyWithZone: z];
+	      o[c + i] = [(*otherObj)(other, objSel, k) copyWithZone: z];
 	      i++;
 	    }
-	  self = [self initWithObjects: os forKeys: ks count: i];
+	  self = [self initWithObjects: o + c forKeys: o count: i];
 #if	!GS_WITH_GC
-	  while (i > 0)
+	  while (i-- > 0)
 	    {
-	      [os[--i] release];
+	      [o[c + i] release];
 	    }
 #endif
 	}
@@ -459,12 +465,13 @@ static SEL	appSel;
 	{
 	  while ((k = (*nxtObj)(e, nxtSel)) != nil)
 	    {
-	      ks[i] = k;
+	      o[i] = k;
-	      os[i] = (*otherObj)(other, objSel, k);
+	      o[c + i] = (*otherObj)(other, objSel, k);
 	      i++;
 	    }
-	  self = [self initWithObjects: os forKeys: ks count: c];
+	  self = [self initWithObjects: o + c forKeys: o count: c];
 	}
+      GS_ENDIDBUF()
     }
   return self;
 }
@@ -656,16 +663,19 @@ static SEL	appSel;
     {
       NSEnumerator	*e = [self keyEnumerator];
       IMP		nxtObj = [e methodForSelector: nxtSel];
-      id		k[c];
       unsigned		i;
+      id		result;
+      GS_BEGINIDBUF(k, c)
 
       for (i = 0; i < c; i++)
 	{
 	  k[i] = (*nxtObj)(e, nxtSel);
 	  NSAssert (k[i], NSInternalInconsistencyException);
 	}
-      return AUTORELEASE([[NSArray_class allocWithZone: NSDefaultMallocZone()]
+      result = [[NSArray_class allocWithZone: NSDefaultMallocZone()]
-	initWithObjects: k count: c]);
+	initWithObjects: k count: c];
+      GS_ENDIDBUF()
+      return AUTORELEASE(result);
     }
 }
 
@@ -684,15 +694,18 @@ static SEL	appSel;
     {
       NSEnumerator	*e = [self objectEnumerator];
       IMP		nxtObj = [e methodForSelector: nxtSel];
-      id		k[c];
+      id		result;
       unsigned		i;
+      GS_BEGINIDBUF(k, c)
 
       for (i = 0; i < c; i++)
 	{
 	  k[i] = (*nxtObj)(e, nxtSel);
 	}
-      return AUTORELEASE([[NSArray_class allocWithZone: NSDefaultMallocZone()]
+      result = [[NSArray_class allocWithZone: NSDefaultMallocZone()]
-	initWithObjects: k count: c]);
+	initWithObjects: k count: c];
+      GS_ENDIDBUF()
+      return AUTORELEASE(result);
     }
 }
 
@@ -715,7 +728,8 @@ static SEL	appSel;
       IMP		myObj = [self methodForSelector: objSel];
       BOOL		(*eqObj)(id, SEL, id);
       id		k;
-      id		a[c];
+      id		result;
+      GS_BEGINIDBUF(a, [self count])
 
       eqObj = (BOOL (*)(id, SEL, id))[anObject methodForSelector: eqSel];
       c = 0;
@@ -729,9 +743,16 @@ static SEL	appSel;
 	    }
 	}
       if (c == 0)
-	return nil;
+	{
-      return AUTORELEASE([[NSArray_class allocWithZone: NSDefaultMallocZone()]
+	  result = nil;
-	initWithObjects: a count: c]);
+	}
+      else
+	{
+	  result = [[NSArray_class allocWithZone: NSDefaultMallocZone()]
+	    initWithObjects: a count: c];
+	}
+      GS_ENDIDBUF()
+      return AUTORELEASE(result);
     }
 }
 
@@ -769,20 +790,28 @@ compareIt(id o1, id o2, void* context)
   else
     {
       unsigned	i;
-      id	obuf[c];
       IMP	myObj = [self methodForSelector: objSel];
+      id	result;
+      GS_BEGINIDBUF(obuf, c)
 
       [keys getObjects: obuf];
       for (i = 0; i < c; i++)
 	{
 	  id o = (*myObj)(self, objSel, obuf[i]);
 
-	  if (o)
+	  if (o == nil)
-	    obuf[i] = o;
+	    {
+	      obuf[i] = marker;
+	    }
 	  else
-	    obuf[i] = marker;
+	    {
+	      obuf[i] = o;
+	    }
 	}
-      return [NSArray_class arrayWithObjects: obuf count: c];
+      result = [[NSArray_class allocWithZone: NSDefaultMallocZone()]
+	initWithObjects: obuf count: c];
+      GS_ENDIDBUF()
+      return AUTORELEASE(result);
     }
 }
 
@@ -977,31 +1006,32 @@ compareIt(id o1, id o2, void* context)
 {
   /* a deep copy */
   unsigned	count = [self count];
-  id		keys[count];
-  id		objects[count];
   NSDictionary	*newDictionary;
   unsigned	i;
   id		key;
   NSEnumerator	*enumerator = [self keyEnumerator];
   IMP		nxtImp = [enumerator methodForSelector: nxtSel];
   IMP		objImp = [self methodForSelector: objSel];
+  GS_BEGINIDBUF(o, count*2)
 
   for (i = 0; (key = (*nxtImp)(enumerator, nxtSel)); i++)
     {
-      keys[i] = key;
+      o[i] = key;
-      objects[i] = (*objImp)(self, objSel, key);
+      o[count + i] = (*objImp)(self, objSel, key);
-      objects[i] = [objects[i] copyWithZone: z];
+      o[count + i] = [o[count + i] copyWithZone: z];
     }
   newDictionary = [[GSDictionaryClass allocWithZone: z] 
-	  initWithObjects: objects
+	  initWithObjects: o + count
-		  forKeys: keys
+		  forKeys: o
 		    count: count];
 #if	!GS_WITH_GC
-  while (i > 0)
+  while (i-- > 0)
     {
-      [objects[--i] release];
+      [o[count + i] release];
     }
 #endif
+  GS_ENDIDBUF()
+
   return newDictionary;
 }
 
@@ -1071,14 +1101,15 @@ compareIt(id o1, id o2, void* context)
 
   if (c > 0)
     {
-      id	keys[c];
       IMP	remObj = [self methodForSelector: remSel];
+      GS_BEGINIDBUF(keys, c)
 
       [keyArray getObjects: keys];
       while (c--)
 	{
 	  (*remObj)(self, remSel, keys[c]);
 	}
+      GS_ENDIDBUF()
     }
 }
 

Source/NSIndexSet.m

@@ -136,6 +136,12 @@ static unsigned posForIndex(GSIArray array, unsigned index)
   unsigned	pos;
   NSRange	r;
 
+  if (NSNotFound - aRange.length < aRange.location)
+    {
+      [NSException raise: NSInvalidArgumentException
+		  format: @"[%@-%@]: Bad range",
+        NSStringFromClass([self class]), NSStringFromSelector(_cmd)];
+    }
   if (_array == 0 || GSIArrayCount(_array) == 0
     || (pos = posForIndex(_array, aRange.location)) >= GSIArrayCount(_array))
     {
@@ -221,6 +227,18 @@ static unsigned posForIndex(GSIArray array, unsigned index)
   unsigned	i = 0;
   NSRange	r;
 
+  if (aBuffer == 0 || aRange == 0)
+    {
+      [NSException raise: NSInvalidArgumentException
+		  format: @"[%@-%@]: nul pointer argument",
+        NSStringFromClass([self class]), NSStringFromSelector(_cmd)];
+    }
+  if (NSNotFound - aRange->length < aRange->location)
+    {
+      [NSException raise: NSInvalidArgumentException
+		  format: @"[%@-%@]: Bad range",
+        NSStringFromClass([self class]), NSStringFromSelector(_cmd)];
+    }
   if (_array == 0 || GSIArrayCount(_array) == 0
     || (pos = posForIndex(_array, aRange->location)) >= GSIArrayCount(_array))
     {
@@ -440,6 +458,12 @@ static unsigned posForIndex(GSIArray array, unsigned index)
   unsigned	p1;
   unsigned	p2;
 
+  if (NSNotFound - aRange.length < aRange.location)
+    {
+      [NSException raise: NSInvalidArgumentException
+		  format: @"[%@-%@]: Bad range",
+        NSStringFromClass([self class]), NSStringFromSelector(_cmd)];
+    }
   if (aRange.length == 0 || _array == 0 || GSIArrayCount(_array) == 0)
     {
       return NO;	// Empty
@@ -549,6 +573,16 @@ static unsigned posForIndex(GSIArray array, unsigned index)
 
 - (void) addIndexesInRange: (NSRange)aRange
 {
+  if (NSNotFound - aRange.length < aRange.location)
+    {
+      [NSException raise: NSInvalidArgumentException
+		  format: @"[%@-%@]: Bad range",
+        NSStringFromClass([self class]), NSStringFromSelector(_cmd)];
+    }
+  if (aRange.length == 0)
+    {
+      return;
+    }
   if (_array == 0)
     {
       _data = (GSIArray)NSZoneMalloc([self zone], sizeof(GSIArray_t));
@@ -560,6 +594,9 @@ static unsigned posForIndex(GSIArray array, unsigned index)
     }
   else
     {
+      unsigned	p1 = posForIndex(_array, aRange.location);
+      unsigned	p2 = posForIndex(_array, NSMaxRange(aRange)-1);
+
       [self notImplemented:_cmd];
     }
 }
@@ -591,6 +628,12 @@ static unsigned posForIndex(GSIArray array, unsigned index)
 
 - (void) removeIndexesInRange: (NSRange)aRange
 {
+  if (NSNotFound - aRange.length < aRange.location)
+    {
+      [NSException raise: NSInvalidArgumentException
+		  format: @"[%@-%@]: Bad range",
+        NSStringFromClass([self class]), NSStringFromSelector(_cmd)];
+    }
   [self notImplemented:_cmd];
 }