[Bug libobjc/61332] New: libobjc unsafe malloc use instead objc_malloc

[max at cert dot cx]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61332

            Bug ID: 61332
           Summary: libobjc unsafe malloc use instead objc_malloc
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: libobjc
          Assignee: unassigned at gcc dot gnu.org
          Reporter: max at cert dot cx

Hi,

I have found interesting examples of dynamic memory allocation use in libobjc

libobjc/selector.c -----------------------------------------
SEL *
sel_copyTypedSelectorList (const char *name, unsigned int
*numberOfReturnedSelectors)
{
  unsigned int count = 0;
  SEL *returnValue = NULL;
  sidx i;

  if (name == NULL)
    {
      if (numberOfReturnedSelectors)
    *numberOfReturnedSelectors = 0;
      return NULL;
    }

  objc_mutex_lock (__objc_runtime_mutex);

  /* Count how many selectors we have.  */
  i = (sidx) objc_hash_value_for_key (__objc_selector_hash, name);
  if (i != 0)
    {
      struct objc_list *selector_list = NULL;
      selector_list = (struct objc_list *) sarray_get_safe
(__objc_selector_array, i);

      /* Count how many selectors we have.  */
      {
    struct objc_list *l;
    for (l = selector_list; l; l = l->tail)
      count++;
      }

      if (count != 0)
    {
      /* Allocate enough memory to hold them.  */
      returnValue = (SEL *)(malloc (sizeof (SEL) * (count + 1)));
<===============

      /* Copy the selectors.  */
      {
        unsigned int j;
        for (j = 0; j < count; j++)
          {
        returnValue[j] = (SEL)(selector_list->head);
<===========================
        selector_list = selector_list->tail;
          }
        returnValue[j] = NULL;
 libobjc/selector.c -----------------------------------------

 or 

 libobjc/ivars.c  -----------------------------------------
 struct objc_ivar ** class_copyIvarList (Class class_, unsigned int
*numberOfReturnedIvars)
{
  unsigned int count = 0;
  struct objc_ivar **returnValue = NULL;
  struct objc_ivar_list* ivar_list;

  if (class_ == Nil  ||  CLS_IS_IN_CONSTRUCTION (class_))
    {
      if (numberOfReturnedIvars)
    *numberOfReturnedIvars = 0;
      return NULL;
    }

  /* Count how many ivars we have.  */
  ivar_list = class_->ivars;
  count = ivar_list->ivar_count;

  if (count != 0)
    {
      unsigned int i = 0;

      /* Allocate enough memory to hold them.  */
      returnValue = (struct objc_ivar **)(malloc (sizeof (struct objc_ivar *) *
(count + 1))); <======

      /* Copy the ivars.  */
      for (i = 0; i < count; i++)
    returnValue[i] = &(ivar_list->ivar_list[i]); <===========================

      returnValue[i] = NULL;
    }
libobjc/ivars.c  -----------------------------------------  

 or

libobjc/protocols.c  -----------------------------------------
Protocol **
objc_copyProtocolList (unsigned int *numberOfReturnedProtocols)
{
  unsigned int count = 0;
  Protocol **returnValue = NULL;
  node_ptr node;

  objc_mutex_lock (__protocols_hashtable_lock);

  /* Count how many protocols we have.  */
  node = objc_hash_next (__protocols_hashtable, NULL);
  while (node)
    {
      count++;
      node = objc_hash_next (__protocols_hashtable, node);
    }

  if (count != 0)
    {
      unsigned int i = 0;

      /* Allocate enough memory to hold them.  */
      returnValue = (Protocol **)(malloc (sizeof (Protocol *) * (count + 1)));
<===========================

      /* Copy the protocols.  */
      node = objc_hash_next (__protocols_hashtable, NULL);
<===========================
      while (node)
    {
      returnValue[i] = node->value;
libobjc/protocols.c  -----------------------------------------


Why malloca() was used instead objc_malloc()? Is there any sensible reason?
In terms of safety, if you can create a properly buffer length (FREE_MEMORY / 2
+1) may end up to crash at this point. 

http://cwe.mitre.org/data/definitions/252.html

Thanks for taking the time.

Maksymilian
http://cifrex.org