Cleanup, dead code removal, some pretty insignificant security stuff that's based on recommendations.

6 files changed, 31 insertions, 33 deletions

diff --git a/node/Buffer.hpp b/node/Buffer.hpp
index 3c4c4015..bc74f048 100644
--- a/node/Buffer.hpp
+++ b/node/Buffer.hpp
@@ -380,12 +380,12 @@ public:
 	}
 
 	/**
-	 * Unconditionally zero buffer's underlying memory
+	 * Unconditionally and securely zero buffer's underlying memory
 	 */
-	inline void zeroAll()
+	inline void burn()
 		throw()
 	{
-		memset(_b,0,sizeof(_b));
+		Utils::burn(_b,sizeof(_b));
 	}
 
 	/**
diff --git a/node/Peer.hpp b/node/Peer.hpp
index af406895..a950b2fc 100644
--- a/node/Peer.hpp
+++ b/node/Peer.hpp
@@ -67,6 +67,8 @@ public:
 	 */
 	Peer();
 
+	~Peer() { Utils::burn(_key,sizeof(_key)); }
+
 	/**
 	 * Construct a new peer
 	 *
diff --git a/node/Topology.cpp b/node/Topology.cpp
index d4df3ce6..d88b64ca 100644
--- a/node/Topology.cpp
+++ b/node/Topology.cpp
@@ -239,13 +239,16 @@ void Topology::_dumpPeers()
 				if (fwrite(buf.data(),buf.size(),1,pd) != 1) {
 					fclose(pd);
 					Utils::rm(pdpath);
+					buf.burn();
 					return;
 				}
 				buf.clear();
+				buf.burn();
 			}
 		} catch ( ... ) {
 			fclose(pd);
 			Utils::rm(pdpath);
+			buf.burn();
 			return;
 		}
 	}
@@ -254,12 +257,15 @@ void Topology::_dumpPeers()
 		if (fwrite(buf.data(),buf.size(),1,pd) != 1) {
 			fclose(pd);
 			Utils::rm(pdpath);
+			buf.burn();
 			return;
 		}
+		buf.burn();
 	}
 
 	fclose(pd);
 	Utils::lockDownFile(pdpath.c_str(),false);
+	buf.burn();
 }
 
 void Topology::_loadPeers()
@@ -301,6 +307,7 @@ void Topology::_loadPeers()
 
 	fclose(pd);
 	Utils::rm(pdpath);
+	buf.burn();
 }
 
 } // namespace ZeroTier
diff --git a/node/Utils.cpp b/node/Utils.cpp
index 95b66337..0b1677e1 100644
--- a/node/Utils.cpp
+++ b/node/Utils.cpp
@@ -395,22 +395,6 @@ std::string Utils::trim(const std::string &s)
 	return s.substr(start,end - start);
 }
 
-void Utils::stdsprintf(std::string &s,const char *fmt,...)
-	throw(std::bad_alloc,std::length_error)
-{
-	char buf[65536];
-	va_list ap;
-
-	va_start(ap,fmt);
-	int n = vsnprintf(buf,sizeof(buf),fmt,ap);
-	va_end(ap);
-
-	if ((n >= (int)sizeof(buf))||(n < 0))
-		throw std::length_error("printf result too large");
-
-	s.append(buf);
-}
-
 unsigned int Utils::snprintf(char *buf,unsigned int len,const char *fmt,...)
 	throw(std::length_error)
 {
diff --git a/node/Utils.hpp b/node/Utils.hpp
index 4f79aa85..fdec54d1 100644
--- a/node/Utils.hpp
+++ b/node/Utils.hpp
@@ -86,6 +86,20 @@ public:
 	}
 
 	/**
+	 * Securely zero memory
+	 *
+	 * This just uses volatile to ensure that it's never optimized out.
+	 */
+	static inline void burn(void *ptr,unsigned int len)
+		throw()
+	{
+		volatile unsigned char *p = (unsigned char *)ptr;
+		volatile unsigned char *e = p + len;
+		while (p != e)
+			*(p++) = (unsigned char)0;
+	}
+
+	/**
 	 * Delete a file
 	 *
 	 * @param path Path to delete
@@ -433,20 +447,11 @@ public:
 	static std::string trim(const std::string &s);
 
 	/**
-	 * Like sprintf, but appends to std::string
-	 *
-	 * @param s String to append to
-	 * @param fmt Printf format string
-	 * @param ... Format arguments
-	 * @throws std::bad_alloc Memory allocation failure
-	 * @throws std::length_error Format + args exceeds internal buffer maximum
-	 */
-	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
 	 *
+	 * This just wraps the local implementation whatever it's called, while
+	 * performing a few other checks and adding exceptions for overflow.
+	 *
 	 * @param buf Buffer to write to
 	 * @param len Length of buffer in bytes
 	 * @param fmt Format string
diff --git a/selftest.cpp b/selftest.cpp
index 4991aef7..9e219db7 100644
--- a/selftest.cpp
+++ b/selftest.cpp
@@ -461,8 +461,8 @@ static int testPacket()
 	unsigned char salsaKey[32],hmacKey[32];
 	Packet a,b;
 
-	a.zeroAll();
-	b.zeroAll();
+	a.burn();
+	b.burn();
 
 	for(unsigned int i=0;i<32;++i) {
 		salsaKey[i] = (unsigned char)rand();