From c4940a5c888d85b4c477b6face5e9a618e64718d Mon Sep 17 00:00:00 2001
From: Guus Sliepen <guus@tinc-vpn.org>
Date: Sun, 30 Sep 2012 13:45:47 +0200
Subject: [PATCH] Add strict checks to hex to binary conversions.

The main goal is to catch misuse of the obsolete PrivateKey and PublicKey
statements.
---
 lib/utils.c         | 10 ++++++----
 lib/utils.h         |  2 +-
 src/net_setup.c     | 15 ++++++++++++---
 src/protocol_auth.c | 15 ++++++++++++---
 src/protocol_key.c  |  8 +++++++-
 5 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index 405097bb..3b221f59 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -32,11 +32,13 @@ static int charhex2bin(char c) {
 		return toupper(c) - 'A' + 10;
 }
 
-
-void hex2bin(char *src, char *dst, int length) {
-	int i;
-	for(i = 0; i < length; i++)
+bool hex2bin(char *src, char *dst, int length) {
+	for(int i = 0; i < length; i++) {
+		if(!isxdigit(src[i * 2]) || !isxdigit(src[i * 2 + 1]))
+			return false;
 		dst[i] = charhex2bin(src[i * 2]) * 16 + charhex2bin(src[i * 2 + 1]);
+	}
+	return true;
 }
 
 void bin2hex(char *src, char *dst, int length) {
diff --git a/lib/utils.h b/lib/utils.h
index f6ff7052..2e4fcf86 100644
--- a/lib/utils.h
+++ b/lib/utils.h
@@ -21,7 +21,7 @@
 #ifndef __TINC_UTILS_H__
 #define __TINC_UTILS_H__
 
-extern void hex2bin(char *src, char *dst, int length);
+extern bool hex2bin(char *src, char *dst, int length);
 extern void bin2hex(char *src, char *dst, int length);
 
 #ifdef HAVE_MINGW
diff --git a/src/net_setup.c b/src/net_setup.c
index eec438a8..a28ab7ad 100644
--- a/src/net_setup.c
+++ b/src/net_setup.c
@@ -66,7 +66,10 @@ bool read_rsa_public_key(connection_t *c) {
 	/* First, check for simple PublicKey statement */
 
 	if(get_config_string(lookup_config(c->config_tree, "PublicKey"), &key)) {
-		BN_hex2bn(&c->rsa_key->n, key);
+		if(BN_hex2bn(&c->rsa_key->n, key) != strlen(key)) {
+			logger(LOG_ERR, "Invalid PublicKey for %s!", c->name);
+			return false;
+		}
 		BN_hex2bn(&c->rsa_key->e, "FFFF");
 		free(key);
 		return true;
@@ -169,8 +172,14 @@ static bool read_rsa_private_key(void) {
 		}
 		myself->connection->rsa_key = RSA_new();
 //		RSA_blinding_on(myself->connection->rsa_key, NULL);
-		BN_hex2bn(&myself->connection->rsa_key->d, key);
-		BN_hex2bn(&myself->connection->rsa_key->n, pubkey);
+		if(BN_hex2bn(&myself->connection->rsa_key->d, key) != strlen(key)) {
+			logger(LOG_ERR, "Invalid PrivateKey for myself!");
+			return false;
+		}
+		if(BN_hex2bn(&myself->connection->rsa_key->n, pubkey) != strlen(pubkey)) {
+			logger(LOG_ERR, "Invalid PublicKey for myself!");
+			return false;
+		}
 		BN_hex2bn(&myself->connection->rsa_key->e, "FFFF");
 		free(key);
 		free(pubkey);
diff --git a/src/protocol_auth.c b/src/protocol_auth.c
index 0ef54682..3bd34a01 100644
--- a/src/protocol_auth.c
+++ b/src/protocol_auth.c
@@ -308,7 +308,10 @@ bool metakey_h(connection_t *c) {
 
 	/* Convert the challenge from hexadecimal back to binary */
 
-	hex2bin(buffer, buffer, len);
+	if(!hex2bin(buffer, buffer, len)) {
+		logger(LOG_ERR, "Got bad %s from %s(%s): %s", "METAKEY", c->name, c->hostname, "invalid key");
+		return false;
+	}
 
 	/* Decrypt the meta key */
 
@@ -426,7 +429,10 @@ bool challenge_h(connection_t *c) {
 
 	/* Convert the challenge from hexadecimal back to binary */
 
-	hex2bin(buffer, c->mychallenge, len);
+	if(!hex2bin(buffer, c->mychallenge, len)) {
+		logger(LOG_ERR, "Got bad %s from %s(%s): %s", "CHALLENGE", c->name, c->hostname, "invalid challenge");
+		return false;
+	}
 
 	c->allow_request = CHAL_REPLY;
 
@@ -480,7 +486,10 @@ bool chal_reply_h(connection_t *c) {
 
 	/* Convert the hash to binary format */
 
-	hex2bin(hishash, hishash, c->outdigest->md_size);
+	if(!hex2bin(hishash, hishash, c->outdigest->md_size)) {
+		logger(LOG_ERR, "Got bad %s from %s(%s): %s", "CHAL_REPLY", c->name, c->hostname, "invalid hash");
+		return false;
+	}
 
 	/* Calculate the hash from the challenge we sent */
 
diff --git a/src/protocol_key.c b/src/protocol_key.c
index 1a184fb8..f2f317de 100644
--- a/src/protocol_key.c
+++ b/src/protocol_key.c
@@ -240,10 +240,16 @@ bool ans_key_h(connection_t *c) {
 		return send_request(to->nexthop->connection, "%s", c->buffer);
 	}
 
+	/* Don't use key material until every check has passed. */
+	from->status.validkey = false;
+
 	/* Update our copy of the origin's packet key */
 	from->outkey = xrealloc(from->outkey, strlen(key) / 2);
 	from->outkeylength = strlen(key) / 2;
-	hex2bin(key, from->outkey, from->outkeylength);
+	if(!hex2bin(key, from->outkey, from->outkeylength)) {
+		logger(LOG_ERR, "Got bad %s from %s(%s): %s", "ANS_KEY", from->name, from->hostname, "invalid key");
+		return true;
+	}
 
 	/* Check and lookup cipher and digest algorithms */
 
-- 
2.39.5