From dbee1b38b3fce2cab93c46157b9144bfab73cf87 Mon Sep 17 00:00:00 2001
From: Adam Ierymenko <adam.ierymenko@gmail.com>
Date: Mon, 29 Jun 2015 10:21:28 -0700
Subject: [PATCH] Fix semantics of std::unique() to actually remove duplicates
(hidden memory leak?)
---
controller/SqliteNetworkController.cpp | 2 +-
node/CertificateOfMembership.cpp | 2 +-
node/InetAddress.cpp | 33 ++++++++++++++++++++++++++
node/InetAddress.hpp | 11 +++++++++
node/Network.cpp | 2 +-
node/NetworkConfig.cpp | 16 +++++++++----
node/NetworkConfig.hpp | 2 ++
node/Node.cpp | 2 +-
service/OneService.cpp | 2 +-
9 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/controller/SqliteNetworkController.cpp b/controller/SqliteNetworkController.cpp
index eb8e399b2..9e4c19086 100644
--- a/controller/SqliteNetworkController.cpp
+++ b/controller/SqliteNetworkController.cpp
@@ -389,7 +389,7 @@ NetworkController::ResultCode SqliteNetworkController::doNetworkConfigRequest(co
allowedEtherTypes.push_back(et);
}
std::sort(allowedEtherTypes.begin(),allowedEtherTypes.end());
- std::unique(allowedEtherTypes.begin(),allowedEtherTypes.end());
+ allowedEtherTypes.erase(std::unique(allowedEtherTypes.begin(),allowedEtherTypes.end()),allowedEtherTypes.end());
std::string allowedEtherTypesCsv;
for(std::vector<int>::const_iterator i(allowedEtherTypes.begin());i!=allowedEtherTypes.end();++i) {
if (allowedEtherTypesCsv.length())
diff --git a/node/CertificateOfMembership.cpp b/node/CertificateOfMembership.cpp
index 15f03cbd6..19ce770a1 100644
--- a/node/CertificateOfMembership.cpp
+++ b/node/CertificateOfMembership.cpp
@@ -140,7 +140,7 @@ void CertificateOfMembership::fromString(const char *s)
}
std::sort(_qualifiers.begin(),_qualifiers.end());
- std::unique(_qualifiers.begin(),_qualifiers.end());
+ _qualifiers.erase(std::unique(_qualifiers.begin(),_qualifiers.end()),_qualifiers.end());
}
bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other) const
diff --git a/node/InetAddress.cpp b/node/InetAddress.cpp
index 940a58b26..83de36d43 100644
--- a/node/InetAddress.cpp
+++ b/node/InetAddress.cpp
@@ -273,6 +273,39 @@ InetAddress InetAddress::broadcast() const
return r;
}
+bool InetAddress::isNetwork() const
+ throw()
+{
+ switch(ss_family) {
+ case AF_INET: {
+ unsigned int bits = netmaskBits();
+ if (bits <= 0)
+ return false;
+ if (bits >= 32)
+ return false;
+ uint32_t ip = Utils::ntoh((uint32_t)reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr);
+ return ((ip & (0xffffffff >> bits)) == 0);
+ }
+ case AF_INET6: {
+ unsigned int bits = netmaskBits();
+ if (bits <= 0)
+ return false;
+ if (bits >= 128)
+ return false;
+ const unsigned char *ip = reinterpret_cast<const unsigned char *>(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr);
+ unsigned int p = bits / 8;
+ if ((ip[p++] & (0xff >> (bits % 8))) != 0)
+ return false;
+ while (p < 16) {
+ if (ip[p++])
+ return false;
+ }
+ return true;
+ }
+ }
+ return false;
+}
+
bool InetAddress::operator==(const InetAddress &a) const
throw()
{
diff --git a/node/InetAddress.hpp b/node/InetAddress.hpp
index 16e3f4d52..35fd43142 100644
--- a/node/InetAddress.hpp
+++ b/node/InetAddress.hpp
@@ -343,6 +343,17 @@ struct InetAddress : public sockaddr_storage
*/
inline void zero() throw() { memset(this,0,sizeof(InetAddress)); }
+ /**
+ * Check whether this is a network/route rather than an IP assignment
+ *
+ * A network is an IP/netmask where everything after the netmask is
+ * zero e.g. 10.0.0.0/8.
+ *
+ * @return True if everything after netmask bits is zero
+ */
+ bool isNetwork() const
+ throw();
+
/**
* @return True if address family is non-zero
*/
diff --git a/node/Network.cpp b/node/Network.cpp
index c072e9785..4414e4d1c 100644
--- a/node/Network.cpp
+++ b/node/Network.cpp
@@ -502,7 +502,7 @@ std::vector<MulticastGroup> Network::_allMulticastGroups() const
if ((_config)&&(_config->enableBroadcast()))
mgs.push_back(Network::BROADCAST);
std::sort(mgs.begin(),mgs.end());
- std::unique(mgs.begin(),mgs.end());
+ mgs.erase(std::unique(mgs.begin(),mgs.end()),mgs.end());
return mgs;
}
diff --git a/node/NetworkConfig.cpp b/node/NetworkConfig.cpp
index 8d682947f..ba4d338bb 100644
--- a/node/NetworkConfig.cpp
+++ b/node/NetworkConfig.cpp
@@ -132,10 +132,18 @@ void NetworkConfig::_fromDictionary(const Dictionary &d)
case AF_INET:
if ((!addr.netmaskBits())||(addr.netmaskBits() > 32))
continue;
+ else if (addr.isNetwork()) {
+ // TODO: add route to network -- this is a route without an IP assignment
+ continue;
+ }
break;
case AF_INET6:
if ((!addr.netmaskBits())||(addr.netmaskBits() > 128))
continue;
+ else if (addr.isNetwork()) {
+ // TODO: add route to network -- this is a route without an IP assignment
+ continue;
+ }
break;
default: // ignore unrecognized address types or junk/empty fields
continue;
@@ -143,9 +151,9 @@ void NetworkConfig::_fromDictionary(const Dictionary &d)
_staticIps.push_back(addr);
}
if (_staticIps.size() > ZT1_MAX_ZT_ASSIGNED_ADDRESSES)
- throw std::invalid_argument("too many ZT-assigned IP addresses");
+ throw std::invalid_argument("too many ZT-assigned IP addresses or routes");
std::sort(_staticIps.begin(),_staticIps.end());
- std::unique(_staticIps.begin(),_staticIps.end());
+ _staticIps.erase(std::unique(_staticIps.begin(),_staticIps.end()),_staticIps.end());
std::vector<std::string> gatewaysSplit(Utils::split(d.get(ZT_NETWORKCONFIG_DICT_KEY_GATEWAYS,"").c_str(),",","",""));
for(std::vector<std::string>::const_iterator gwstr(gatewaysSplit.begin());gwstr!=gatewaysSplit.end();++gwstr) {
@@ -163,7 +171,7 @@ void NetworkConfig::_fromDictionary(const Dictionary &d)
}
}
std::sort(_activeBridges.begin(),_activeBridges.end());
- std::unique(_activeBridges.begin(),_activeBridges.end());
+ _activeBridges.erase(std::unique(_activeBridges.begin(),_activeBridges.end()),_activeBridges.end());
std::vector<std::string> relaysSplit(Utils::split(d.get(ZT_NETWORKCONFIG_DICT_KEY_RELAYS,"").c_str(),",","",""));
for(std::vector<std::string>::const_iterator r(relaysSplit.begin());r!=relaysSplit.end();++r) {
@@ -177,7 +185,7 @@ void NetworkConfig::_fromDictionary(const Dictionary &d)
}
}
std::sort(_relays.begin(),_relays.end());
- std::unique(_relays.begin(),_relays.end());
+ _relays.erase(std::unique(_relays.begin(),_relays.end()),_relays.end());
_com.fromString(d.get(ZT_NETWORKCONFIG_DICT_KEY_CERTIFICATE_OF_MEMBERSHIP,std::string()));
}
diff --git a/node/NetworkConfig.hpp b/node/NetworkConfig.hpp
index 75395fd53..5c7cdd7c0 100644
--- a/node/NetworkConfig.hpp
+++ b/node/NetworkConfig.hpp
@@ -78,9 +78,11 @@ namespace ZeroTier {
#define ZT_NETWORKCONFIG_DICT_KEY_DESC "d"
// IP/bits[,IP/bits,...]
+// Note that IPs that end in all zeroes are routes with no assignment in them.
#define ZT_NETWORKCONFIG_DICT_KEY_IPV4_STATIC "v4s"
// IP/bits[,IP/bits,...]
+// Note that IPs that end in all zeroes are routes with no assignment in them.
#define ZT_NETWORKCONFIG_DICT_KEY_IPV6_STATIC "v6s"
// serialized CertificateOfMembership
diff --git a/node/Node.cpp b/node/Node.cpp
index 850114341..8cdc6d62b 100644
--- a/node/Node.cpp
+++ b/node/Node.cpp
@@ -251,7 +251,7 @@ ZT1_ResultCode Node::processBackgroundTasks(uint64_t now,volatile uint64_t *next
// Attempt to contact network preferred relays that we don't have direct links to
std::sort(networkRelays.begin(),networkRelays.end());
- std::unique(networkRelays.begin(),networkRelays.end());
+ networkRelays.erase(std::unique(networkRelays.begin(),networkRelays.end()),networkRelays.end());
for(std::vector< std::pair<Address,InetAddress> >::const_iterator nr(networkRelays.begin());nr!=networkRelays.end();++nr) {
if (nr->second) {
SharedPtr<Peer> rp(RR->topology->getPeer(nr->first));
diff --git a/service/OneService.cpp b/service/OneService.cpp
index 212525460..7532bf750 100644
--- a/service/OneService.cpp
+++ b/service/OneService.cpp
@@ -864,7 +864,7 @@ public:
for(unsigned int i=0;i<nwc->assignedAddressCount;++i)
newAssignedIps.push_back(InetAddress(nwc->assignedAddresses[i]));
std::sort(newAssignedIps.begin(),newAssignedIps.end());
- std::unique(newAssignedIps.begin(),newAssignedIps.end());
+ newAssignedIps.erase(std::unique(newAssignedIps.begin(),newAssignedIps.end()),newAssignedIps.end());
for(std::vector<InetAddress>::iterator ip(newAssignedIps.begin());ip!=newAssignedIps.end();++ip) {
if (!std::binary_search(assignedIps.begin(),assignedIps.end(),*ip))
t->second->addIp(*ip);