From 03c139ff898f51f96870c8524c096288e87bd9fb Mon Sep 17 00:00:00 2001 From: bmenrigh Date: Mon, 16 Mar 2009 21:31:57 +0000 Subject: [PATCH] Fixed (removed) the use of strtok in TargetGroup::parse_expr by using strchr() per David's suggestion. strtok uses static (global) state to track the string it is parsing. In this case, load_exclude was also using strtok and calling parse_expr which was wiping out the previous strtok state. This introduce two bugs, first, only the first exclude on a line would be loaded from the exclude file, and second, there was an invalid access into free()'d memory in load_exclude (found with Valgrind). The use of strtok should be highly discouraged because these types of bugs are so easy to introduce. --- CHANGELOG | 5 +++++ TargetGroup.cc | 8 ++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 88f860843..021c9b296 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,10 @@ # Nmap Changelog ($Id$); -*-text-*- +o Fixed a strtok issue between load_exclude and + TargetGroup::parse_expr that caused only the first exclude on + a line to be loaded as well as an invalid read into free()'d + memory in load_exclude(). [Brandon, David] + Nmap 4.85BETA4 [2009-3-15] o Added two new SMB/MSRPC NSE scripts by Ron Bowes: diff --git a/TargetGroup.cc b/TargetGroup.cc index f8ef8f6e9..1a071060c 100644 --- a/TargetGroup.cc +++ b/TargetGroup.cc @@ -186,8 +186,12 @@ int TargetGroup::parse_expr(const char * const target_expr, int af) { addy[0] = r = hostexp; /* First we break the expression up into the four parts of the IP address + the optional '/mask' */ - target_net = strtok(hostexp, "/"); - s = strtok(NULL, ""); /* find the end of the token from hostexp */ + target_net = hostexp; + s = strchr(hostexp, '/'); /* Find the slash if there is one */ + if (s) { + *s = '\0'; /* Make sure target_net is terminated before the /## */ + s++; /* Point s at the netmask */ + } netmask = ( s ) ? atoi(s) : 32; if ((int) netmask < 0 || netmask > 32) { error("Illegal netmask value (%d), must be /0 - /32 . Assuming /32 (one host)", netmask);