From cef06973a24cc728d2903a63034f5524a95d6ba3 Mon Sep 17 00:00:00 2001 From: Yang Tse Date: Fri, 30 Oct 2009 16:21:56 +0000 Subject: 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. --- Makefile.inc | 4 +++- ares.h | 2 +- ares_parse_txt_reply.c | 53 ++++++++++++++++++++++++++++++++------------------ 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 1fe1129..e383ebb 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -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 \ diff --git a/ares.h b/ares.h index 35038d2..593a145 100644 --- a/ares.h +++ b/ares.h @@ -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; }; diff --git a/ares_parse_txt_reply.c b/ares_parse_txt_reply.c index c99ee72..6950912 100644 --- a/ares_parse_txt_reply.c +++ b/ares_parse_txt_reply.c @@ -1,4 +1,7 @@ +/* $Id$ */ + /* Copyright (C) 2009 Jakub Hrozek +* Copyright (C) 2009 Yang Tse * * 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; } - free (hostname); - free (rr_name); + 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; } -- cgit v1.2.3