From a4fc71afb661fea265832003854a73d7f6d509cc Mon Sep 17 00:00:00 2001 From: david Date: Wed, 18 Nov 2009 01:17:59 +0000 Subject: [PATCH] Return the last host before skipping an octet in an IPv4 range exclude group. Not doing this was the cause of off-by-one errors that led to assertion failures and, potentially, excluded hosts being scanned. Here is the comment I added: /* The decision to skip a range was based on the address that came immediately before what our current array contains now. For example, if we have just handed out 0.0.0.0 from the the range 0-5.0.0.0, and we're asked to skip the first octet, we want to advance to 1.0.0.0. But 1.0.0.0 is what is in the current array right now, because TargetGroup::get_next_host advances the array after returning an address. If we didn't step back we would erroneously skip ahead to 2.0.0.0. */ --- CHANGELOG | 9 +++++++++ TargetGroup.cc | 13 ++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index af3608764..2477ccbcf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,14 @@ # Nmap Changelog ($Id$); -*-text-*- +o Fixed an error in the handling of exclude groups that used IPv4 + ranges. Si Stransky reported the problem and provided a number of + useful test cases in http://seclists.org/nmap-dev/2009/q4/276. The + error caused various assertion failures along the lines of + TargetGroup.cc:465: int + TargetGroup::get_next_host(sockaddr_storage*, size_t*): + Assertion `ipsleft > 1' failed. + [David] + o Add a new script, db2-info.nse, that enhances DB2 database instance detection. The script provides detection when version probes fail, but will default to the value provided the version probes if that value is more precise. The diff --git a/TargetGroup.cc b/TargetGroup.cc index dcf4237fd..749587c5f 100644 --- a/TargetGroup.cc +++ b/TargetGroup.cc @@ -351,6 +351,15 @@ int TargetGroup::skip_range(_octet_nums octet) { if (targets_type != IPV4_RANGES) return -1; + /* The decision to skip a range was based on the address that came immediately + before what our current array contains now. For example, if we have just + handed out 0.0.0.0 from the the range 0-5.0.0.0, and we're asked to skip + the first octet, we want to advance to 1.0.0.0. But 1.0.0.0 is what is in + the current array right now, because TargetGroup::get_next_host advances + the array after returning an address. If we didn't step back we would + erroneously skip ahead to 2.0.0.0. */ + return_last_host(); + switch (octet) { case FIRST_OCTET: oct = 0; @@ -386,9 +395,7 @@ int TargetGroup::skip_range(_octet_nums octet) { current[i] = 0; } - /* we actually don't skip the current, it was accounted for - * by get_next_host */ - ipsleft -= hosts_skipped - 1; + ipsleft -= hosts_skipped; return hosts_skipped; }