Security tokens: wipe APDU buffers throughout their lifetime

EMV keyfile cleanup did not cover APDU response storage, temporary
copies, or transport buffers on exceptions. Wipe responses on clear,
replacement, destruction and growth, avoid redundant response and PAN
copies, and install transport cleanup guards before use.

Add standalone checks for erasure before release, aliasing, allocation
failures, exception unwinding and const TLV input.

Tests/response_apdu_test.cpp

@@ -0,0 +1,158 @@
+// Standalone cleanup checks using synthetic data; no smart card is required.
+// c++ -std=c++11 -O2 -Isrc Tests/response_apdu_test.cpp \
+//     src/Common/ResponseAPDU.cpp src/Common/TLVParser.cpp -o /tmp/response_apdu_test
+#include "Common/ResponseAPDU.h"
+#include "Common/TLVParser.h"
+#include <cstdio>
+#include <cstdlib>
+#include <new>
+#include <type_traits>
+#include <utility>
+
+using namespace VeraCrypt;
+
+static const void *watched = NULL;
+static size_t watchedSize = 0;
+static unsigned int wipesChecked = 0;
+static bool failNextAllocation = false;
+
+static void Require(bool condition, const char *message)
+{
+	if (!condition)
+	{
+		std::fprintf(stderr, "FAIL: %s\n", message);
+		std::abort();
+	}
+}
+
+void *operator new(size_t size)
+{
+	if (failNextAllocation)
+	{
+		failNextAllocation = false;
+		throw std::bad_alloc();
+	}
+	void *p = std::malloc(size ? size : 1);
+	if (!p) throw std::bad_alloc();
+	return p;
+}
+
+void operator delete(void *p) noexcept
+{
+	if (p && p == watched)
+	{
+		// Inspect while storage is still allocated, before forwarding to free().
+		const unsigned char *bytes = static_cast<const unsigned char *>(p);
+		for (size_t i = 0; i < watchedSize; ++i)
+			Require(bytes[i] == 0, "response storage released without erasure");
+		watched = NULL;
+		++wipesChecked;
+	}
+	std::free(p);
+}
+
+#if __cplusplus >= 201402L
+void operator delete(void *p, size_t) noexcept { ::operator delete(p); }
+#endif
+
+static void Watch(const ResponseAPDU& response)
+{
+	Require(watched == NULL && !response.getData().empty(), "invalid cleanup observation");
+	watched = response.getData().data();
+	watchedSize = response.getData().size();
+}
+
+int main()
+{
+	static_assert(std::is_same<decltype(std::declval<const ResponseAPDU&>().getData()),
+		const vector<uint8>&>::value, "getData must not copy response bytes");
+	const vector<uint8> original(37, 0x5a);
+	const vector<uint8> suffix(7, 0x23);
+
+	{
+		ResponseAPDU response(original, 0x9000);
+		Watch(response);
+		response.clear();
+		Require(response.getData().empty() && response.getSW() == 0, "clear changed semantics");
+		response.appendData(suffix);
+		Require(response.getData() == suffix, "reuse after clear");
+		// The original tail must also have been wiped, even though size is now smaller.
+	}
+	{
+		ResponseAPDU response(original, 0x9000);
+		const vector<uint8> extra(response.getData().capacity() + 1, 0x23);
+		Watch(response);
+		response.appendData(extra);
+		Require(watched == NULL, "growth did not release old storage");
+		Require(response.getData().size() == original.size() + extra.size(), "append size");
+		for (size_t i = 0; i < response.getData().size(); ++i)
+			Require(response.getData()[i] == (i < original.size() ? 0x5a : 0x23), "append contents");
+		Watch(response);
+	}
+	{
+		ResponseAPDU response(original, 0x9000);
+		response.appendData(response.getData());
+		Require(response.getData() == vector<uint8>(original.size() * 2, 0x5a), "aliased append");
+		response.clear();
+		response.appendData(suffix);
+		response.appendData(response.getData());
+		Require(response.getData() == vector<uint8>(suffix.size() * 2, 0x23), "aliased append with capacity");
+		Watch(response);
+	}
+	{
+		ResponseAPDU source(original, 0x9000), target(suffix, 0x6a82);
+		Watch(target);
+		target = source;
+		Require(watched == NULL && target.getData() == original && target.getSW() == 0x9000, "copy assignment");
+		ResponseAPDU copy(target);
+		target.clear();
+		Require(copy.getData() == original, "copy must own its storage");
+		ResponseAPDU& alias = copy;
+		copy = alias;
+		Require(copy.getData() == original, "self assignment");
+		Watch(copy);
+	}
+	{
+		ResponseAPDU source(original, 0x9000), target(suffix, 0x6a82);
+		vector<uint8> encoded = source.getBytes();
+		Watch(target);
+		target.setBytes(encoded);
+		Require(watched == NULL && target.getData() == original && target.getSW() == 0x9000, "response round trip");
+		ResponseAPDU alias(encoded, 0);
+		alias.setBytes(alias.getData());
+		Require(alias.getData() == original && alias.getSW() == 0x9000, "aliased setBytes");
+		Watch(target);
+		target.setBytes(vector<uint8>(1, 0));
+		Require(watched == NULL && target.getData().empty() && target.getSW() == 0, "short response");
+	}
+	{
+		ResponseAPDU response(original, 0x9000), target(suffix, 0x6a82);
+		const vector<uint8> extra(response.getData().capacity() + 1, 0x23);
+		failNextAllocation = true;
+		try { response.appendData(extra); Require(false, "expected allocation failure"); }
+		catch (const std::bad_alloc&) {}
+		Require(response.getData() == original && response.getSW() == 0x9000, "failed growth changed response");
+		failNextAllocation = true;
+		try { target = response; Require(false, "expected allocation failure"); }
+		catch (const std::bad_alloc&) {}
+		Require(target.getData() == suffix && target.getSW() == 0x6a82, "failed assignment changed response");
+		Watch(response);
+	}
+	try
+	{
+		ResponseAPDU response(original, 0x9000);
+		Watch(response);
+		throw 1;
+	}
+	catch (int) {}
+	{
+		const uint8 encoded[] = { 0x5a, 0x02, 0x12, 0x34 };
+		ResponseAPDU response(vector<uint8>(encoded, encoded + sizeof(encoded)), 0x9000);
+		shared_ptr<TLVNode> node = TLVParser::TLV_Parse(response.getData().data(), response.getData().size());
+		response.clear();
+		Require(node->Tag == 0x5a && node->Value->size() == 2
+			&& (*node->Value)[0] == 0x12 && (*node->Value)[1] == 0x34, "parser retained borrowed storage");
+	}
+	Require(watched == NULL && wipesChecked == 10, "missing cleanup observations");
+	std::puts("PASS: response ownership, erasure before release, replacement, growth, aliasing, allocation failure, unwinding and const TLV input");
+}

src/Common/EMVCard.cpp

@@ -151,7 +151,6 @@ namespace VeraCrypt
 		bool 								hasBeenReset = false;
 		CommandAPDU 						command;
 		ResponseAPDU 						response;
-		vector<uint8> 						responseData;
 		shared_ptr<TLVNode> 				rootNode;
 		shared_ptr<TLVNode> 				fciNode;
 		shared_ptr<TLVNode> 				dfNameNode;
@@ -218,8 +217,7 @@ namespace VeraCrypt
 						}
 						if (response.getSW() == SW_NO_ERROR && response.getData().size() > 0)
 						{
-							responseData = response.getData();
-							rootNode = TLVParser::TLV_Parse(responseData.data(), responseData.size());
+							rootNode = TLVParser::TLV_Parse(response.getData().data(), response.getData().size());
 							fciNode = TLVParser::TLV_Find(rootNode, EMV_FCI_TAG);
 							if (fciNode && fciNode->Subs->size() >= 2)
 							{
@@ -261,11 +259,9 @@ namespace VeraCrypt
 													m_reader->ApduProcessData(command, response);
 													if (response.getSW() == SW_NO_ERROR && response.getData().size() > 0)
 													{
-														responseData = response.getData();
-
 														try
 														{
-															templateNode = TLVParser::TLV_Parse(responseData.data(), responseData.size());
+															templateNode = TLVParser::TLV_Parse(response.getData().data(), response.getData().size());
 															if (templateNode && templateNode->Tag == EMV_TEMPLATE_TAG && templateNode->Subs->size() >= 1)
 															{
 																for (size_t i = 0; i < templateNode->Subs->size(); i++)
@@ -342,10 +338,15 @@ namespace VeraCrypt
 		shared_ptr<TLVNode>			issuerPublicKeyCertNode;
 		CommandAPDU					command;
 		ResponseAPDU				response;
-		vector<uint8>				responseData;
 
+		if (!iccCert.empty())
+			burn(iccCert.data(), iccCert.size());
 		iccCert.clear();
+		if (!issuerCert.empty())
+			burn(issuerCert.data(), issuerCert.size());
 		issuerCert.clear();
+		if (!cplcData.empty())
+			burn(cplcData.data(), cplcData.size());
 		cplcData.clear();
 
 		if (m_iccCert.size() && m_issuerCert.size() && m_cplcData.size())
@@ -395,11 +396,9 @@ namespace VeraCrypt
 								m_reader->ApduProcessData(command, response);
 								if (response.getSW() == SW_NO_ERROR && response.getData().size() > 0)
 								{
-									responseData = response.getData();
-
 									try
 									{
-										rootNode = TLVParser::TLV_Parse(responseData.data(), responseData.size());
+										rootNode = TLVParser::TLV_Parse(response.getData().data(), response.getData().size());
 									}
 									catch(TLVException)
 									{
@@ -410,6 +409,8 @@ namespace VeraCrypt
 									if (iccPublicKeyCertNode && iccPublicKeyCertNode->Value->size() > 0)
 									{
 										iccFound = true;
+										if (!iccCert.empty())
+											burn(iccCert.data(), iccCert.size());
 										iccCert = *iccPublicKeyCertNode->Value.get();
 									}
 
@@ -417,6 +418,8 @@ namespace VeraCrypt
 									if (issuerPublicKeyCertNode && issuerPublicKeyCertNode->Value->size() > 0)
 									{
 										issuerFound = true;
+										if (!issuerCert.empty())
+											burn(issuerCert.data(), issuerCert.size());
 										issuerCert = *issuerPublicKeyCertNode->Value.get();
 									}
 								}
@@ -456,12 +459,12 @@ namespace VeraCrypt
 		bool						panFound		= false;
 		bool						aidSelected		= false;
 		vector<uint8>				EMVCardAid;
-		vector<uint8>				panData;
+		uint8 panData[2] = { 0 };
+		finally_do_arg (uint8 *, panData, { burn (finally_arg, 2); });
 		shared_ptr<TLVNode>			rootNode;
 		shared_ptr<TLVNode>			panNode;
 		CommandAPDU					command;
 		ResponseAPDU				response;
-		vector<uint8>				responseData;
 
 		lastPANDigits = L"";
 
@@ -501,11 +504,9 @@ namespace VeraCrypt
 							m_reader->ApduProcessData(command, response);
 							if (response.getSW() == SW_NO_ERROR && response.getData().size() > 0)
 							{
-								responseData = response.getData();
-
 								try
 								{
-									rootNode = TLVParser::TLV_Parse(responseData.data(), responseData.size());
+									rootNode = TLVParser::TLV_Parse(response.getData().data(), response.getData().size());
 								}
 								catch(TLVException)
 								{
@@ -516,10 +517,10 @@ namespace VeraCrypt
 								if (panNode && panNode->Value->size() >= 8)
 								{
 									panFound = true;
-									panData = *panNode->Value.get();
-									panData = vector<uint8>(panData.rbegin(), panData.rbegin() + 2); // only interested in last digits
-									std::swap(panData[0], panData[1]);
-									lastPANDigits = ArrayToHexWideString(panData.data(), (int) panData.size());
+									// Only copy the last digits, leaving the full PAN in the wiping TLV node.
+									panData[0] = (*panNode->Value)[panNode->Value->size() - 2];
+									panData[1] = panNode->Value->back();
+									lastPANDigits = ArrayToHexWideString(panData, (int) sizeof(panData));
 								}
 							}
 						}
@@ -528,9 +529,6 @@ namespace VeraCrypt
 			}
 		}
 		
-		if (panData.size())
-			burn(panData.data(), panData.size());
-
 		if (!aidSelected)
 			throw EMVSelectAIDFailed();
 

src/Common/ResponseAPDU.cpp

@@ -1,33 +1,18 @@
 #include "ResponseAPDU.h"
+#include "Tcdefs.h"
+#include <stdexcept>
 #include <string.h>
 
 using namespace std;
 
 namespace VeraCrypt
 {
-	uint16 BytesToUInt16(const vector<uint8>& buff)
-	{
-		uint16 value = 0;
-		for (uint16 i = 0; i < buff.size(); i++)
-		{
-			value <<= 8;
-			value |= (uint16)buff.at(i);
-		}
-
-		return value;
-	}
-
-	void AppendData (vector<uint8>& buffer, const uint8* pbData, size_t cbData)
-	{
-		size_t orgSize = buffer.size ();
-		buffer.resize (orgSize + cbData);
-		memcpy (buffer.data () + orgSize, pbData, cbData);
-	}
-
 	/*********************************************************************************/
 
 	void ResponseAPDU::clear()
 	{
+		if (!m_data.empty())
+			burn(m_data.data(), m_data.size());
 		m_data.clear();
 		m_SW = 0;
 	}
@@ -36,10 +21,28 @@ namespace VeraCrypt
 	{
 	}
 
-	ResponseAPDU::ResponseAPDU(const vector<uint8>& data, uint16 SW)
+	ResponseAPDU::ResponseAPDU(const vector<uint8>& data, uint16 SW) : m_data(data), m_SW(SW)
 	{
-		m_data = data;
-		m_SW = SW;
+	}
+
+	ResponseAPDU::ResponseAPDU(const ResponseAPDU& other) : m_data(other.m_data), m_SW(other.m_SW)
+	{
+	}
+
+	ResponseAPDU& ResponseAPDU::operator=(const ResponseAPDU& other)
+	{
+		if (this != &other)
+		{
+			ResponseAPDU replacement(other);
+			m_data.swap(replacement.m_data);
+			m_SW = other.m_SW;
+		}
+		return *this;
+	}
+
+	ResponseAPDU::~ResponseAPDU()
+	{
+		clear();
 	}
 
 	uint32 ResponseAPDU::getNr()
@@ -47,7 +50,7 @@ namespace VeraCrypt
 		return (uint32)m_data.size();
 	}
 
-	const vector<uint8> ResponseAPDU::getData()
+	const vector<uint8>& ResponseAPDU::getData() const
 	{
 		return m_data;
 	}
@@ -67,13 +70,17 @@ namespace VeraCrypt
 		return m_SW;
 	}
 
-	const vector<uint8> ResponseAPDU::getBytes()
+	vector<uint8> ResponseAPDU::getBytes()
 	{
 		vector<uint8> apdu;
-
-		AppendData(apdu, m_data.data(), m_data.size());
-		apdu.push_back((uint8)getSW1());
-		apdu.push_back((uint8)getSW2());
+		if (m_data.size() > apdu.max_size() - 2)
+			throw length_error("APDU response too large");
+		// Allocate the complete output before copying sensitive data.
+		apdu.resize(m_data.size() + 2);
+		if (!m_data.empty())
+			memcpy(apdu.data(), m_data.data(), m_data.size());
+		apdu[m_data.size()] = (uint8)getSW1();
+		apdu[m_data.size() + 1] = (uint8)getSW2();
 
 		return apdu;
 	}
@@ -85,7 +92,34 @@ namespace VeraCrypt
 
 	void ResponseAPDU::appendData(const uint8* data, size_t dataLen)
 	{
-		AppendData(m_data, data, dataLen);
+		if (dataLen == 0)
+			return;
+		if (dataLen > m_data.max_size() - m_data.size())
+			throw length_error("APDU response too large");
+
+		size_t oldSize = m_data.size();
+		size_t newSize = oldSize + dataLen;
+		if (newSize > m_data.capacity())
+		{
+			// vector growth would otherwise release the previous contents without wiping.
+			vector<uint8> replacement;
+			size_t capacity = m_data.capacity();
+			if (capacity <= m_data.max_size() / 2 && capacity * 2 > newSize)
+				replacement.reserve(capacity * 2);
+			replacement.resize(newSize);
+			if (oldSize > 0)
+				memcpy(replacement.data(), m_data.data(), oldSize);
+			memcpy(replacement.data() + oldSize, data, dataLen);
+			// Copy the appended bytes before erasing, as data may refer to m_data.
+			if (oldSize > 0)
+				burn(m_data.data(), oldSize);
+			m_data.swap(replacement);
+		}
+		else
+		{
+			m_data.resize(newSize);
+			memcpy(m_data.data() + oldSize, data, dataLen);
+		}
 	}
 
 	void ResponseAPDU::setSW(uint16 SW)
@@ -95,17 +129,14 @@ namespace VeraCrypt
 
 	void ResponseAPDU::setBytes(const vector<uint8>& bytes)
 	{
-		clear();
+		ResponseAPDU replacement;
 		if (bytes.size() >= 2)
 		{
-			vector<uint8> SWBytes;
-			m_data.resize(bytes.size() - 2);
-			SWBytes.resize(2);
-
-			memcpy(m_data.data(), bytes.data(), bytes.size() - 2);
-			memcpy(SWBytes.data(), bytes.data() + bytes.size() - 2, 2);
-			m_SW = BytesToUInt16(SWBytes);
+			replacement.appendData(bytes.data(), bytes.size() - 2);
+			replacement.m_SW = (static_cast<uint16>(bytes[bytes.size() - 2]) << 8) | bytes.back();
 		}
+		m_data.swap(replacement.m_data);
+		m_SW = replacement.m_SW;
 	}
 }
 

src/Common/ResponseAPDU.h

@@ -20,10 +20,14 @@ namespace VeraCrypt
 		ResponseAPDU();
 
 		ResponseAPDU(const vector<uint8>& data, uint16 SW);
+		ResponseAPDU(const ResponseAPDU& other);
+		ResponseAPDU& operator=(const ResponseAPDU& other);
+		~ResponseAPDU();
 
 		uint32 getNr();
 
-		const vector<uint8> getData();
+		// The returned data remains owned by this response and is invalidated by mutation.
+		const vector<uint8>& getData() const;
 
 		uint8 getSW1();
 
@@ -31,7 +35,8 @@ namespace VeraCrypt
 
 		uint16 getSW();
 
-		const vector<uint8> getBytes();
+		// The caller owns this serialized copy and is responsible for erasing it.
+		vector<uint8> getBytes();
 
 		void setSW(uint16 SW);
 		void setBytes(const vector<uint8>& bytes);
@@ -41,4 +46,4 @@ namespace VeraCrypt
 	};
 };
 
-#endif // TC_HEADER_Common_ResponseAPDU
+#endif // TC_HEADER_Common_ResponseAPDU

src/Common/SCardReader.cpp

@@ -1,5 +1,6 @@
 #include "SCardReader.h"
 #include "PCSCException.h"
+#include "Platform/Finally.h"
 
 #include <locale>
 
@@ -399,7 +400,9 @@ namespace VeraCrypt
 		size_t indexOfLcData = 0;
 
 		vector<uint8> pbSendBuffer;
+		finally_do_arg (vector<uint8> *, &pbSendBuffer, { if (!finally_arg->empty()) burn (finally_arg->data(), finally_arg->size()); });
 		vector<uint8> pbRecvBuffer;
+		finally_do_arg (vector<uint8> *, &pbRecvBuffer, { if (!finally_arg->empty()) burn (finally_arg->data(), finally_arg->size()); });
 		DWORD cbSendLength = 0;
 		DWORD cbRecvLength = 0;
 	
@@ -639,9 +642,6 @@ namespace VeraCrypt
 
 	end:
 
-		burn(pbSendBuffer.data(), pbSendBuffer.size());
-		burn(pbRecvBuffer.data(), pbRecvBuffer.size());
-
 		if (lRet != SCARD_S_SUCCESS)
 			throw PCSCException(lRet);
 	}

src/Common/TLVParser.cpp

@@ -46,7 +46,7 @@ namespace VeraCrypt
 	}
 
 	/* Parsing one TLV node */
-	shared_ptr<TLVNode> TLVParser::TLV_Parse_One(uint8* buf, size_t size)
+	shared_ptr<TLVNode> TLVParser::TLV_Parse_One(const uint8* buf, size_t size)
 	{
 		size_t index = 0;
 		size_t i = 0;
@@ -205,7 +205,7 @@ namespace VeraCrypt
 	}
 
 	/* Parsing TLV from a buffer and constructing TLV structure */
-	shared_ptr<TLVNode> TLVParser::TLV_Parse(uint8* buf, size_t size)
+	shared_ptr<TLVNode> TLVParser::TLV_Parse(const uint8* buf, size_t size)
 	{
 		shared_ptr<TLVNode> node = TLV_Parse_One(buf, size);
 		TLV_Parse_Sub(node);

src/Common/TLVParser.h

@@ -40,7 +40,7 @@ namespace VeraCrypt
 		static uint16 CheckBit(uint8 value, int bit);
 
 		/* Parsing one TLV node */
-		static shared_ptr<TLVNode> TLV_Parse_One(uint8* buf, size_t size);
+		static shared_ptr<TLVNode> TLV_Parse_One(const uint8* buf, size_t size);
 
 		/* Parsing all TLV nodes */
 		static int TLV_Parse_SubNodes(shared_ptr<TLVNode> parent);
@@ -54,7 +54,7 @@ namespace VeraCrypt
 	public:
 
 		/* Parsing TLV from a buffer and constructing TLV structure */
-		static shared_ptr<TLVNode> TLV_Parse(uint8* buf, size_t size);
+		static shared_ptr<TLVNode> TLV_Parse(const uint8* buf, size_t size);
 
 		/* Finding a TLV node with a particular tag */
 		static shared_ptr<TLVNode> TLV_Find(shared_ptr<TLVNode> node, uint16 tag);