Improve code security posture by replacing sprintf with a safer function.

node/Address.hpp

@@ -32,7 +32,9 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+
 #include <string>
+
 #include "Utils.hpp"
 #include "MAC.hpp"
 #include "Constants.hpp"
@@ -198,7 +200,7 @@ public:
 	inline std::string toString() const
 	{
 		char buf[16];
-		sprintf(buf,"%.10llx",(unsigned long long)_a);
+		Utils::snprintf(buf,sizeof(buf),"%.10llx",(unsigned long long)_a);
 		return std::string(buf);
 	};
 

node/Demarc.cpp

@@ -68,15 +68,14 @@ Demarc::~Demarc()
 }
 
 std::string Demarc::describe(Demarc::Port p)
-	throw()
 {
 	char buf[64];
 	switch ((DemarcPortType)(((uint64_t)p) >> 60)) {
 		case PORT_TYPE_UDP_SOCKET_V4:
-			sprintf(buf,"udp/4/%d",(int)((uint64_t)p & 0xffff));
+			Utils::snprintf(buf,sizeof(buf),"udp/4/%d",(int)((uint64_t)p & 0xffff));
 			return std::string(buf);
 		case PORT_TYPE_UDP_SOCKET_V6:
-			sprintf(buf,"udp/6/%d",(int)((uint64_t)p & 0xffff));
+			Utils::snprintf(buf,sizeof(buf),"udp/6/%d",(int)((uint64_t)p & 0xffff));
 			return std::string(buf);
 		case PORT_TYPE_LOCAL_ETHERNET:
 			return std::string("ethernet");

node/Demarc.hpp

@@ -83,8 +83,7 @@ public:
 	 * @param p Port
 	 * @return Human-readable description of port
 	 */
-	static std::string describe(Port p)
-		throw();
+	static std::string describe(Port p);
 
 	/**
 	 * @param p Port to check

node/EthernetTap.cpp

@@ -36,6 +36,7 @@
 #include "RuntimeEnvironment.hpp"
 #include "Utils.hpp"
 #include "Mutex.hpp"
+#include "Utils.hpp"
 
 // ff:ff:ff:ff:ff:ff with no ADI
 static const ZeroTier::MulticastGroup _blindWildcardMulticastGroup(ZeroTier::MAC(0xff),0);
@@ -99,22 +100,22 @@ private:
 	inline void _findCmd(int id,const char *name)
 	{
 		char tmp[4096];
-		sprintf(tmp,"/sbin/%s",name);
+		ZeroTier::Utils::snprintf(tmp,sizeof(tmp),"/sbin/%s",name);
 		if (ZeroTier::Utils::fileExists(tmp)) {
 			_paths[id] = tmp;
 			return;
 		}
-		sprintf(tmp,"/usr/sbin/%s",name);
+		ZeroTier::Utils::snprintf(tmp,sizeof(tmp),"/usr/sbin/%s",name);
 		if (ZeroTier::Utils::fileExists(tmp)) {
 			_paths[id] = tmp;
 			return;
 		}
-		sprintf(tmp,"/bin/%s",name);
+		ZeroTier::Utils::snprintf(tmp,sizeof(tmp),"/bin/%s",name);
 		if (ZeroTier::Utils::fileExists(tmp)) {
 			_paths[id] = tmp;
 			return;
 		}
-		sprintf(tmp,"/usr/bin/%s",name);
+		ZeroTier::Utils::snprintf(tmp,sizeof(tmp),"/usr/bin/%s",name);
 		if (ZeroTier::Utils::fileExists(tmp)) {
 			_paths[id] = tmp;
 			return;
@@ -178,8 +179,8 @@ EthernetTap::EthernetTap(
 		int devno = 0;
 		struct stat sbuf;
 		do {
-			sprintf(ifr.ifr_name,"zt%d",devno++);
-			sprintf(procpath,"/proc/sys/net/ipv4/conf/%s",ifr.ifr_name);
+			Utils::snprintf(ifr.ifr_name,sizeof(ifr.ifr_name),"zt%d",devno++);
+			Utils::snprintf(procpath,sizeof(procpath),"/proc/sys/net/ipv4/conf/%s",ifr.ifr_name);
 		} while (stat(procpath,&sbuf) == 0);
 	}
 
@@ -292,12 +293,12 @@ EthernetTap::EthernetTap(
 
 	// Open the first available device (ones in use will fail with resource busy)
 	for(int i=0;i<256;++i) {
-		sprintf(devpath,"/dev/zt%d",i);
+		Utils::snprintf(devpath,sizeof(devpath),"/dev/zt%d",i);
 		if (stat(devpath,&tmp))
 			throw std::runtime_error("no more TAP devices available");
 		_fd = ::open(devpath,O_RDWR);
 		if (_fd > 0) {
-			sprintf(_dev,"zt%d",i);
+			Utils::snprintf(_dev,sizeof(_dev),"zt%d",i);
 			break;
 		}
 	}
@@ -316,8 +317,8 @@ EthernetTap::EthernetTap(
 	}
 
 	// Configure MAC address and MTU, bring interface up
-	sprintf(ethaddr,"%.2x:%.2x:%.2x:%.2x:%.2x:%.2x",(int)mac[0],(int)mac[1],(int)mac[2],(int)mac[3],(int)mac[4],(int)mac[5]);
-	sprintf(mtustr,"%u",mtu);
+	Utils::snprintf(ethaddr,sizeof(ethaddr),"%.2x:%.2x:%.2x:%.2x:%.2x:%.2x",(int)mac[0],(int)mac[1],(int)mac[2],(int)mac[3],(int)mac[4],(int)mac[5]);
+	Utils::snprintf(mtustr,sizeof(mtustr),"%u",mtu);
 	long cpid;
 	if ((cpid = (long)vfork()) == 0) {
 		execl(ifconfig,ifconfig,_dev,"lladdr",ethaddr,"mtu",mtustr,"up",(const char *)0);
@@ -895,7 +896,7 @@ EthernetTap::EthernetTap(
 	// If we have a device, configure it
 	if (_myDeviceInstanceId.length() > 0) {
 		char tmps[4096];
-		unsigned int tmpsl = sprintf_s(tmps,"%.2X-%.2X-%.2X-%.2X-%.2X-%.2X",(unsigned int)mac.data[0],(unsigned int)mac.data[1],(unsigned int)mac.data[2],(unsigned int)mac.data[3],(unsigned int)mac.data[4],(unsigned int)mac.data[5]) + 1;
+		unsigned int tmpsl = Utils::snprintf(tmps,sizeof(tmps),"%.2X-%.2X-%.2X-%.2X-%.2X-%.2X",(unsigned int)mac.data[0],(unsigned int)mac.data[1],(unsigned int)mac.data[2],(unsigned int)mac.data[3],(unsigned int)mac.data[4],(unsigned int)mac.data[5]) + 1;
 		RegSetKeyValueA(nwAdapters,mySubkeyName.c_str(),"NetworkAddress",REG_SZ,tmps,tmpsl);
 		RegSetKeyValueA(nwAdapters,mySubkeyName.c_str(),"MAC",REG_SZ,tmps,tmpsl);
 		DWORD tmp = mtu;
@@ -961,7 +962,7 @@ EthernetTap::EthernetTap(
 
 	// Open the tap, which is in this weird Windows analog of /dev
 	char tapPath[4096];
-	sprintf_s(tapPath,"\\\\.\\Global\\%s.tap",_myDeviceInstanceId.c_str());
+	Utils::snprintf(tapPath,sizeof(tapPath),"\\\\.\\Global\\%s.tap",_myDeviceInstanceId.c_str());
 	_tap = CreateFileA(tapPath,GENERIC_READ|GENERIC_WRITE,0,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_SYSTEM|FILE_FLAG_OVERLAPPED,NULL);
 	if (_tap == INVALID_HANDLE_VALUE)
 		throw std::runtime_error("unable to open tap in \\\\.\\Global\\ namespace");

node/Filter.cpp

@@ -186,7 +186,7 @@ bool Filter::Rule::operator()(unsigned int etype,const void *data,unsigned int l
 								break;
 							default: {
 								char foo[128];
-								sprintf(foo,"unrecognized IPv6 header type %d",(int)nextHeader);
+								Utils::snprintf(foo,sizeof(foo),"unrecognized IPv6 header type %d",(int)nextHeader);
 								throw std::invalid_argument(foo);
 							}
 						}
@@ -215,11 +215,11 @@ std::string Filter::Rule::toString() const
 			s.push_back('*');
 			break;
 		case 1:
-			sprintf(buf,"%u",_etherType.start);
+			Utils::snprintf(buf,sizeof(buf),"%u",_etherType.start);
 			s.append(buf);
 			break;
 		default:
-			sprintf(buf,"%u-%u",_etherType.start,_etherType.end);
+			Utils::snprintf(buf,sizeof(buf),"%u-%u",_etherType.start,_etherType.end);
 			s.append(buf);
 			break;
 	}
@@ -229,11 +229,11 @@ std::string Filter::Rule::toString() const
 			s.push_back('*');
 			break;
 		case 1:
-			sprintf(buf,"%u",_protocol.start);
+			Utils::snprintf(buf,sizeof(buf),"%u",_protocol.start);
 			s.append(buf);
 			break;
 		default:
-			sprintf(buf,"%u-%u",_protocol.start,_protocol.end);
+			Utils::snprintf(buf,sizeof(buf),"%u-%u",_protocol.start,_protocol.end);
 			s.append(buf);
 			break;
 	}
@@ -243,11 +243,11 @@ std::string Filter::Rule::toString() const
 			s.push_back('*');
 			break;
 		case 1:
-			sprintf(buf,"%u",_port.start);
+			Utils::snprintf(buf,sizeof(buf),"%u",_port.start);
 			s.append(buf);
 			break;
 		default:
-			sprintf(buf,"%u-%u",_port.start,_port.end);
+			Utils::snprintf(buf,sizeof(buf),"%u-%u",_port.start,_port.end);
 			s.append(buf);
 			break;
 	}
@@ -269,7 +269,7 @@ Filter::Filter(const char *s)
 			++fn;
 		} catch (std::invalid_argument &exc) {
 			char tmp[256];
-			sprintf(tmp,"invalid rule at index %u: %s",fn,exc.what());
+			Utils::snprintf(tmp,sizeof(tmp),"invalid rule at index %u: %s",fn,exc.what());
 			throw std::invalid_argument(tmp);
 		}
 	}

node/InetAddress.cpp

@@ -32,6 +32,7 @@
 
 #include "Constants.hpp"
 #include "InetAddress.hpp"
+#include "Utils.hpp"
 
 namespace ZeroTier {
 
@@ -66,7 +67,7 @@ std::string InetAddress::toString() const
 #else
 			if (inet_ntop(AF_INET,(const void *)&(_sa.sin.sin_addr.s_addr),buf,sizeof(buf))) {
 #endif
-				sprintf(buf2,"%s/%u",buf,(unsigned int)ntohs(_sa.sin.sin_port));
+				Utils::snprintf(buf2,sizeof(buf2),"%s/%u",buf,(unsigned int)ntohs(_sa.sin.sin_port));
 				return std::string(buf2);
 			}
 			break;
@@ -76,7 +77,7 @@ std::string InetAddress::toString() const
 #else
 			if (inet_ntop(AF_INET6,(const void *)&(_sa.sin6.sin6_addr.s6_addr),buf,sizeof(buf))) {
 #endif
-				sprintf(buf2,"%s/%u",buf,(unsigned int)ntohs(_sa.sin6.sin6_port));
+				Utils::snprintf(buf2,sizeof(buf2),"%s/%u",buf,(unsigned int)ntohs(_sa.sin6.sin6_port));
 				return std::string(buf2);
 			}
 			break;

node/MAC.hpp

@@ -30,8 +30,9 @@
 
 #include <stdio.h>
 #include <stdlib.h>
-#include "Array.hpp"
+
 #include "Constants.hpp"
+#include "Array.hpp"
 #include "Utils.hpp"
 
 namespace ZeroTier {
@@ -150,7 +151,7 @@ public:
 	inline std::string toString() const
 	{
 		char tmp[32];
-		sprintf(tmp,"%.2x:%.2x:%.2x:%.2x:%.2x:%.2x",(int)data[0],(int)data[1],(int)data[2],(int)data[3],(int)data[4],(int)data[5]);
+		Utils::snprintf(tmp,sizeof(tmp),"%.2x:%.2x:%.2x:%.2x:%.2x:%.2x",(int)data[0],(int)data[1],(int)data[2],(int)data[3],(int)data[4],(int)data[5]);
 		return std::string(tmp);
 	}
 };

node/MulticastGroup.hpp

@@ -106,7 +106,7 @@ public:
 	inline std::string toString() const
 	{
 		char buf[64];
-		sprintf(buf,"%.2x:%.2x:%.2x:%.2x:%.2x:%.2x/%.8lx",(unsigned int)_mac.data[0],(unsigned int)_mac.data[1],(unsigned int)_mac.data[2],(unsigned int)_mac.data[3],(unsigned int)_mac.data[4],(unsigned int)_mac.data[5],(unsigned long)_adi);
+		Utils::snprintf(buf,sizeof(buf),"%.2x:%.2x:%.2x:%.2x:%.2x:%.2x/%.8lx",(unsigned int)_mac.data[0],(unsigned int)_mac.data[1],(unsigned int)_mac.data[2],(unsigned int)_mac.data[3],(unsigned int)_mac.data[4],(unsigned int)_mac.data[5],(unsigned long)_adi);
 		return std::string(buf);
 	}
 

node/Network.cpp

@@ -140,7 +140,7 @@ SharedPtr<Network> Network::newInstance(const RuntimeEnvironment *renv,uint64_t
 	throw(std::runtime_error)
 {
 	char tag[32];
-	sprintf(tag,"%.16llx",(unsigned long long)id);
+	Utils::snprintf(tag,sizeof(tag),"%.16llx",(unsigned long long)id);
 
 	// We construct Network via a static method to ensure that it is immediately
 	// wrapped in a SharedPtr<>. Otherwise if there is traffic on the Ethernet

node/Network.hpp

@@ -107,7 +107,7 @@ public:
 		inline void setNetworkId(uint64_t id)
 		{
 			char buf[32];
-			sprintf(buf,"%.16llx",(unsigned long long)id);
+			Utils::snprintf(buf,sizeof(buf),"%.16llx",(unsigned long long)id);
 			(*this)["nwid"] = buf;
 		}
 
@@ -141,9 +141,9 @@ public:
 		inline void setTimestamp(uint64_t ts,uint64_t maxDelta)
 		{
 			char foo[32];
-			sprintf(foo,"%llu",(unsigned long long)ts);
+			Utils::snprintf(foo,sizeof(foo),"%llu",(unsigned long long)ts);
 			(*this)["ts"] = foo;
-			sprintf(foo,"%llu",(unsigned long long)maxDelta);
+			Utils::snprintf(foo,sizeof(foo),"%llu",(unsigned long long)maxDelta);
 			(*this)["~ts"] = foo;
 		}
 
@@ -237,7 +237,7 @@ public:
 			if (contains("name"))
 				return get("name");
 			char buf[32];
-			sprintf(buf,"%.16llx",(unsigned long long)networkId());
+			Utils::snprintf(buf,sizeof(buf),"%.16llx",(unsigned long long)networkId());
 			return std::string(buf);
 		}
 
@@ -373,7 +373,7 @@ public:
 	inline std::string toString()
 	{
 		char buf[64];
-		sprintf(buf,"%.16llx",(unsigned long long)_id);
+		Utils::snprintf(buf,sizeof(buf),"%.16llx",(unsigned long long)_id);
 		return std::string(buf);
 	}
 

node/Node.cpp

@@ -590,7 +590,7 @@ public:
 	char vs[32];
 	_VersionStringMaker()
 	{
-		sprintf(vs,"%d.%d.%d",(int)ZEROTIER_ONE_VERSION_MAJOR,(int)ZEROTIER_ONE_VERSION_MINOR,(int)ZEROTIER_ONE_VERSION_REVISION);
+		Utils::snprintf(vs,sizeof(vs),"%d.%d.%d",(int)ZEROTIER_ONE_VERSION_MAJOR,(int)ZEROTIER_ONE_VERSION_MINOR,(int)ZEROTIER_ONE_VERSION_REVISION);
 	}
 	~_VersionStringMaker() {}
 };

node/PacketDecoder.cpp

@@ -635,9 +635,9 @@ bool PacketDecoder::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *_r,const
 				request["meta"] = std::string((const char *)field(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST_IDX_DICT,dictLen),dictLen);
 			request["type"] = "netconf-request";
 			request["peerId"] = peer->identity().toString(false);
-			sprintf(tmp,"%llx",(unsigned long long)nwid);
+			Utils::snprintf(tmp,sizeof(tmp),"%llx",(unsigned long long)nwid);
 			request["nwid"] = tmp;
-			sprintf(tmp,"%llx",(unsigned long long)packetId());
+			Utils::snprintf(tmp,sizeof(tmp),"%llx",(unsigned long long)packetId());
 			request["requestId"] = tmp;
 			//TRACE("to netconf:\n%s",request.toString().c_str());
 			_r->netconfService->send(request);

node/Peer.hpp

@@ -356,7 +356,7 @@ public:
 	{
 		if ((_vMajor)||(_vMinor)||(_vRevision)) {
 			char tmp[32];
-			sprintf(tmp,"%u.%u.%u",_vMajor,_vMinor,_vRevision);
+			Utils::snprintf(tmp,sizeof(tmp),"%u.%u.%u",_vMajor,_vMinor,_vRevision);
 			return std::string(tmp);
 		}
 		return std::string("?");

node/Utils.cpp

@@ -464,7 +464,7 @@ std::string Utils::toRfc1123(uint64_t t64)
 #else
 	gmtime_r(&utc,&t);
 #endif
-	sprintf(buf,"%3s, %02d %3s %4d %02d:%02d:%02d GMT",DAY_NAMES[t.tm_wday],t.tm_mday,MONTH_NAMES[t.tm_mon],t.tm_year + 1900,t.tm_hour,t.tm_min,t.tm_sec);
+	Utils::snprintf(buf,sizeof(buf),"%3s, %02d %3s %4d %02d:%02d:%02d GMT",DAY_NAMES[t.tm_wday],t.tm_mday,MONTH_NAMES[t.tm_mon],t.tm_year + 1900,t.tm_hour,t.tm_min,t.tm_sec);
 	return std::string(buf);
 }
 
@@ -653,4 +653,22 @@ void Utils::stdsprintf(std::string &s,const char *fmt,...)
 	s.append(buf);
 }
 
+unsigned int Utils::snprintf(char *buf,unsigned int len,const char *fmt,...)
+	throw(std::length_error)
+{
+	va_list ap;
+
+	va_start(ap,fmt);
+	int n = (int)vsnprintf(buf,len,fmt,ap);
+	va_end(ap);
+
+	if ((n >= (int)len)||(n < 0)) {
+		if (len)
+			buf[len - 1] = (char)0;
+		throw std::length_error("buf[] overflow in Utils::snprintf");
+	}
+
+	return (unsigned int)n;
+}
+
 } // namespace ZeroTier

node/Utils.hpp

@@ -536,6 +536,18 @@ public:
 	static void stdsprintf(std::string &s,const char *fmt,...)
 		throw(std::bad_alloc,std::length_error);
 
+	/**
+	 * Variant of snprintf that is portable and throws an exception
+	 *
+	 * @param buf Buffer to write to
+	 * @param len Length of buffer in bytes
+	 * @param fmt Format string
+	 * @param ... Format arguments
+	 * @throws std::length_error buf[] too short (buf[] will still be left null-terminated)
+	 */
+	static unsigned int snprintf(char *buf,unsigned int len,const char *fmt,...)
+		throw(std::length_error);
+
 	/**
 	 * Count the number of bits set in an integer
 	 *

selftest.cpp

@@ -376,32 +376,6 @@ static int testOther()
 	return 0;
 }
 
-static int testRateLimiter()
-{
-	RateLimiter limiter;
-	RateLimiter::Limit limit;
-
-	std::cout << "[ratelimiter] preload: 10000.0, rate: 1000.0/sec, max: 15000.0, min: -7500.0" << std::endl;
-	limit.bytesPerSecond = 1000.0;
-	limit.maxBalance = 15000.0;
-	limit.minBalance = -7500.0;
-	limiter.init(10000.0);
-	for(int i=0;i<25;++i) {
-		Thread::sleep(100);
-		std::cout << "[ratelimiter] delayed 0.1s, gate(1000.0): " << (limiter.gate(limit,1000.0) ? "OK" : "BLOCK");
-		std::cout << " (new balance afterwords: " << limiter.balance() << ")" << std::endl;
-	}
-	std::cout << "[ratelimiter] delaying 15s..." << std::endl;
-	Thread::sleep(15000);
-	for(int i=0;i<20;++i) {
-		Thread::sleep(1000);
-		std::cout << "[ratelimiter] delayed 1s, gate(2000.0): " << (limiter.gate(limit,2000.0) ? "OK" : "BLOCK");
-		std::cout << " (new balance afterwords: " << limiter.balance() << ")" << std::endl;
-	}
-
-	return 0;
-}
-
 #ifdef __WINDOWS__
 int _tmain(int argc, _TCHAR* argv[])
 #else
@@ -417,7 +391,6 @@ int main(int argc,char **argv)
 	r |= testPacket();
 	r |= testOther();
 	r |= testIdentity();
-	r |= testRateLimiter();
 
 	if (r)
 		std::cout << std::endl << "SOMETHING FAILED!" << std::endl;