1 files changed, 54 insertions, 29 deletions

diff --git a/src/modules/m_dnsbl.cpp b/src/modules/m_dnsbl.cpp
index 376ada344..98b121a0d 100644
--- a/src/modules/m_dnsbl.cpp
+++ b/src/modules/m_dnsbl.cpp
@@ -43,8 +43,16 @@ class DNSBLConfEntry
 		unsigned long duration;
 		unsigned int bitmask;
 		unsigned char records[256];
-		unsigned long stats_hits, stats_misses;
-		DNSBLConfEntry(): type(A_BITMASK),duration(86400),bitmask(0),stats_hits(0), stats_misses(0) {}
+		unsigned long stats_hits, stats_misses, stats_errors;
+		DNSBLConfEntry()
+			: type(A_BITMASK)
+			, duration(86400)
+			, bitmask(0)
+			, stats_hits(0)
+			, stats_misses(0)
+			, stats_errors(0)
+		{
+		}
 };
 
 
@@ -78,41 +86,55 @@ class DNSBLResolver : public DNS::Request
 		if (!them || them->client_sa != theirsa)
 			return;
 
+		int i = countExt.get(them);
+		if (i)
+			countExt.set(them, i - 1);
+
+		// The DNSBL reply must contain an A result.
 		const DNS::ResourceRecord* const ans_record = r->FindAnswerOfType(DNS::QUERY_A);
 		if (!ans_record)
+		{
+			ConfEntry->stats_errors++;
+			ServerInstance->SNO.WriteGlobalSno('d', "%s returned an result with no IPv4 address.",
+				ConfEntry->name.c_str());
 			return;
+		}
 
-		// All replies should be in 127.0.0.0/8
-		if (ans_record->rdata.compare(0, 4, "127.") != 0)
+		// The DNSBL reply must be a valid IPv4 address.
+		in_addr resultip;
+		if (inet_pton(AF_INET, ans_record->rdata.c_str(), &resultip) != 1)
 		{
-			ServerInstance->SNO.WriteGlobalSno('d', "DNSBL: %s returned address outside of acceptable subnet 127.0.0.0/8: %s", ConfEntry->domain.c_str(), ans_record->rdata.c_str());
-			ConfEntry->stats_misses++;
+			ConfEntry->stats_errors++;
+			ServerInstance->SNO.WriteGlobalSno('d', "%s returned an invalid IPv4 address: %s",
+				ConfEntry->name.c_str(), ans_record->rdata.c_str());
 			return;
 		}
 
-		int i = countExt.get(them);
-		if (i)
-			countExt.set(them, i - 1);
-
-		// Now we calculate the bitmask: 256*(256*(256*a+b)+c)+d
+		// The DNSBL reply should be in the 127.0.0.0/8 range.
+		if ((resultip.s_addr & 0xFF) != 127)
+		{
+			ConfEntry->stats_errors++;
+			ServerInstance->SNO.WriteGlobalSno('d', "%s returned an IPv4 address which is outside of the 127.0.0.0/8 subnet: %s",
+				ConfEntry->name.c_str(), ans_record->rdata.c_str());
+			return;
+		}
 
-		unsigned int bitmask = 0, record = 0;
 		bool match = false;
-		in_addr resultip;
-
-		inet_pton(AF_INET, ans_record->rdata.c_str(), &resultip);
-
+		unsigned int result = 0;
 		switch (ConfEntry->type)
 		{
 			case DNSBLConfEntry::A_BITMASK:
-				bitmask = resultip.s_addr >> 24; /* Last octet (network byte order) */
-				bitmask &= ConfEntry->bitmask;
-				match = (bitmask != 0);
-			break;
+			{
+				result = (resultip.s_addr >> 24) & ConfEntry->bitmask;
+				match = (result != 0);
+				break;
+			}
 			case DNSBLConfEntry::A_RECORD:
-				record = resultip.s_addr >> 24; /* Last octet */
-				match = (ConfEntry->records[record] == 1);
-			break;
+			{
+				result = resultip.s_addr >> 24;
+				match = (ConfEntry->records[result] == 1);
+				break;
+			}
 		}
 
 		if (match)
@@ -212,7 +234,7 @@ class DNSBLResolver : public DNS::Request
 			}
 
 			ServerInstance->SNO.WriteGlobalSno('d', "Connecting user %s (%s) detected as being on the '%s' DNS blacklist with result %d",
-				them->GetFullRealHost().c_str(), them->GetIPString().c_str(), ConfEntry->name.c_str(), (ConfEntry->type==DNSBLConfEntry::A_BITMASK) ? bitmask : record);
+				them->GetFullRealHost().c_str(), them->GetIPString().c_str(), ConfEntry->name.c_str(), result);
 		}
 		else
 			ConfEntry->stats_misses++;
@@ -234,6 +256,7 @@ class DNSBLResolver : public DNS::Request
 			return;
 		}
 
+		ConfEntry->stats_errors++;
 		ServerInstance->SNO.WriteGlobalSno('d', "An error occurred whilst checking whether %s (%s) is on the '%s' DNS blacklist: %s",
 			them->GetFullRealHost().c_str(), them->GetIPString().c_str(), ConfEntry->name.c_str(), this->manager->GetErrorStr(q->error).c_str());
 	}
@@ -456,20 +479,22 @@ class ModuleDNSBL : public Module, public Stats::EventListener
 		if (stats.GetSymbol() != 'd')
 			return MOD_RES_PASSTHRU;
 
-		unsigned long total_hits = 0, total_misses = 0;
-
+		unsigned long total_hits = 0;
+		unsigned long total_misses = 0;
+		unsigned long total_errors = 0;
 		for (std::shared_ptr<DNSBLConfEntry> e : DNSBLConfEntries)
 		{
 			total_hits += e->stats_hits;
 			total_misses += e->stats_misses;
+			total_errors += e->stats_errors;
 
-			stats.AddRow(304, "DNSBLSTATS DNSbl \"" + e->name + "\" had " +
-					ConvToStr(e->stats_hits) + " hits and " + ConvToStr(e->stats_misses) + " misses");
+			stats.AddRow(304, InspIRCd::Format("DNSBLSTATS \"%s\" had %lu hits, %lu misses, and %lu errors",
+				e->name.c_str(), e->stats_hits, e->stats_misses, e->stats_errors));
 		}
 
 		stats.AddRow(304, "DNSBLSTATS Total hits: " + ConvToStr(total_hits));
 		stats.AddRow(304, "DNSBLSTATS Total misses: " + ConvToStr(total_misses));
-
+		stats.AddRow(304, "DNSBLSTATS Total errors: " + ConvToStr(total_errors));
 		return MOD_RES_PASSTHRU;
 	}
 };