Commit d4ff44d5 authored by Yang Tse's avatar Yang Tse
Browse files

In no particular order, changed/fixed all of the following in 

ares_parse_txt_reply() current version:

- Fixed a couple of potential double free's.

- Fixed memory leaks upon out of memory condition.

- Fixed pointer arithmetic.

- Setting ntxtreply to zero upon entry for all failure cases.

- Changed data type to size_t for variables substr_len, str_len and
  the length member of ares_txt_reply struct.

- Avoided a couple of memcpy() calls.

- Changed i data type to unsigned int to prevent compiler warnings.

- Adjusted a comment.

- Use ARES_SUCCESS literal for successfull completion.

- Added CVS Id tag.
parent fff706d7

ares/Makefile.inc

+3 −1
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ CSOURCES = ares__close_sockets.c \ 
  ares_parse_ptr_reply.c		\

  ares_parse_srv_reply.c		\

  ares_parse_txt_reply.c		\

  ares_free_txt_reply.c                 \

  ares_process.c			\

  ares_query.c				\

  ares_search.c				\
@@ -83,6 +82,7 @@ MANPAGES = ares_cancel.3 \ 
  ares_parse_ns_reply.3			\

  ares_parse_ptr_reply.3		\

  ares_parse_srv_reply.3		\

  ares_parse_txt_reply.3		\

  ares_process.3			\

  ares_query.3				\

  ares_save_options.3			\
@@ -117,6 +117,7 @@ HTMLPAGES = ares_cancel.html \ 
  ares_parse_ns_reply.html		\

  ares_parse_ptr_reply.html		\

  ares_parse_srv_reply.html		\

  ares_parse_txt_reply.html		\

  ares_process.html			\

  ares_query.html			\

  ares_save_options.html		\
@@ -151,6 +152,7 @@ PDFPAGES = ares_cancel.pdf \ 
  ares_parse_ns_reply.pdf		\

  ares_parse_ptr_reply.pdf		\

  ares_parse_srv_reply.pdf		\

  ares_parse_txt_reply.pdf		\

  ares_process.pdf			\

  ares_query.pdf			\

  ares_save_options.pdf			\

ares/ares.h

+1 −1
Original line number Diff line number Diff line
@@ -437,7 +437,7 @@ struct ares_srv_reply { 
};



struct ares_txt_reply {

  unsigned int  length;

  size_t         length;  /* length excludes null termination */

  unsigned char *txt;

};

ares/ares_parse_txt_reply.c

+34 −19
Original line number Diff line number Diff line 
/* $Id$ */



/* Copyright (C) 2009 Jakub Hrozek <jhrozek@redhat.com>

*  Copyright (C) 2009 Yang Tse <yangsita@gmail.com>

*

* Permission to use, copy, modify, and distribute this

* software and its documentation for any purpose and without
@@ -38,18 +41,21 @@ int 
ares_parse_txt_reply (const unsigned char *abuf, int alen,

                      struct ares_txt_reply **txt_out, int *ntxtreply)

{

  unsigned char substr_len = 0;

  unsigned char str_len = 0;

  unsigned int qdcount, ancount;

  size_t substr_len, str_len;

  unsigned int qdcount, ancount, i;

  const unsigned char *aptr;

  const unsigned char *strptr;

  int status, i, rr_type, rr_class, rr_len;

  int status, rr_type, rr_class, rr_len;

  long len;

  char *hostname = NULL, *rr_name = NULL;

  struct ares_txt_reply *txt = NULL;



  /* Set *txt_out to NULL for all failure cases. */

  *txt_out = NULL;



  /* Same with *nsrvreply. */

  *ntxtreply = 0;



  /* Give up if abuf doesn't have room for a header. */

  if (alen < HFIXEDSZ)

    return ARES_EBADRESP;
@@ -82,11 +88,16 @@ ares_parse_txt_reply (const unsigned char *abuf, int alen, 
      free (hostname);

      return ARES_ENOMEM;

    }

  /* Zero out so we can safely free txt.txt even if NULL */

  memset(txt, 0, ancount * sizeof (struct ares_txt_reply));



  /* Initialize ares_txt_reply array */

  for (i = 0; i < ancount; i++)

    {

      txt[i].txt = NULL;

      txt[i].length = 0;

    }



  /* Examine each answer resource record (RR) in turn. */

  for (i = 0; i < (int) ancount; i++)

  for (i = 0; i < ancount; i++)

    {

      /* Decode the RR up to the data field. */

      status = ares_expand_name (aptr, abuf, alen, &rr_name, &len);
@@ -116,18 +127,17 @@ ares_parse_txt_reply (const unsigned char *abuf, int alen, 
           * substrings contained therein.

           */



          /* Realloc would be expensive, compute the total length */

          txt[i].length = 0;

          /* Compute total length to allow a single memory allocation */

          strptr = aptr;

          while (strptr < (aptr + rr_len))

            {

              memcpy ((void *) &substr_len, strptr, sizeof (unsigned char));

              substr_len = (unsigned char)*strptr;

              txt[i].length += substr_len;

              strptr += substr_len + 1;

            }



          /* Including null byte */

          txt[i].txt = malloc (sizeof (unsigned char) * (txt[i].length + 1));

          txt[i].txt = malloc (txt[i].length + 1);

          if (txt[i].txt == NULL)

            {

              status = ARES_ENOMEM;
@@ -135,15 +145,13 @@ ares_parse_txt_reply (const unsigned char *abuf, int alen, 
            }



          /* Step through the list of substrings, concatenating them */

          substr_len = 0;

          str_len = 0;

          strptr = aptr;

          while (strptr < (aptr + rr_len))

            {

              memcpy ((void *) &substr_len, strptr, sizeof (unsigned char));

              substr_len = (unsigned char)*strptr;

              strptr++;

              memcpy ((void *) txt[i].txt + str_len, strptr,

                      sizeof (unsigned char) * substr_len);

              memcpy ((char *) txt[i].txt + str_len, strptr, substr_len);

              str_len += substr_len;

              strptr += substr_len;

            }
@@ -159,18 +167,25 @@ ares_parse_txt_reply (const unsigned char *abuf, int alen, 
      rr_name = NULL;

    }



  if (hostname);

    free (hostname);

  if (rr_name);

    free (rr_name);



  /* clean up on error */

  if (status != ARES_SUCCESS)

    {

      ares_free_txt_reply(txt, ancount);

    for (i = 0; i < ancount; i++)

      {

        if (txt[i].txt)

          free (txt[i].txt);

      }

      return status;

    }



  /* everything looks fine, return the data */

  *txt_out = txt;

  *ntxtreply = ancount;

  return 0;



  return ARES_SUCCESS;

}