Fix bug with libzrtp zrtp_signaling_hash_set()

The function would silently not accept the imported zrtp-hash-value with "buffer too small" in the debug output. Modified-by: Travis Cross <[email protected]> Signed-off-by: Travis Cross <[email protected]>
connexo · Feb 11, 2013 · 7503d8a · 7503d8a
1 parent f311f81
commit 7503d8a
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 20 deletions.
diff --git a/libs/libzrtp/doc/manuals/changelog.dox b/libs/libzrtp/doc/manuals/changelog.dox
@@ -34,6 +34,8 @@
 ***\subsection v120_bugs Bug fixes
   *- fixed bug when ZRTP forces enrolled endpoints to re-render SAS when sashash is empty.
   *- other minor bug fixes and improvements
+  *- fixed bug when zrtp_signaling_hash_set() silently not accepted imported zrtp-hash-value with 
+     "buffer too small" debug output.
 
 
 ****************************************************************************************************

diff --git a/libs/libzrtp/include/zrtp.h b/libs/libzrtp/include/zrtp.h
@@ -82,6 +82,9 @@
  * \ingroup zrtp_main_init 
  */
 
+/** Length of "zrtp-hash-value", RFC 6189 sec 8. @sa zrtp_signaling_hash_get(); */
+#define ZRTP_SIGN_ZRTP_HASH_LENGTH	(ZRTP_MESSAGE_HASH_SIZE*2)
+
 /**
  * \brief Enumeration for ZRTP Licensing modes
  * \ingroup zrtp_main_init 
@@ -798,7 +801,7 @@ zrtp_status_t  zrtp_process_srtcp( zrtp_stream_t *stream,
  * 
  * \param stream - stream for operating with;
  * \param hash_buff - signaling hash buffer. Function accepts string, not a binary value!;
- * \param hash_buff_length - signaling hash length in bytes (must be 64 bytes);
+ * \param hash_buff_length - signaling hash length in bytes, must be ZRTP_SIGN_ZRTP_HASH_LENGTH bytes;
  * \return:
  *  - zrtp_status_ok if the operation finished successfully
  *  - one of the errors otherwise
@@ -819,8 +822,8 @@ zrtp_status_t zrtp_signaling_hash_set( zrtp_stream_t* stream,
  *
  * \param stream - stream for operating with
  * \param hash_buff - buffer for storing signaling hash. Function returns already parsed hex string.
- *      String is null-terminated.
- * \param hash_buff_length - buffer length in bytes (not shorter than 65 bytes)
+ *      String is null-terminated. Buffer must be at least ZRTP_SIGN_ZRTP_HASH_LENGTH bytes length.
+ * \param hash_buff_length - buffer length in bytes, non less  than ZRTP_SIGN_ZRTP_HASH_LENGTH bytes.
  * \return:
  *  - zrtp_status_ok if the operation finished successfully
  *  - one of the errors otherwise

diff --git a/libs/libzrtp/src/zrtp.c b/libs/libzrtp/src/zrtp.c
@@ -437,8 +437,6 @@ zrtp_status_t zrtp_stream_attach(zrtp_session_t *session, zrtp_stream_t** stream
 	ZSTR_SET_EMPTY(new_stream->cc.peer_hmackey);
 	ZSTR_SET_EMPTY(new_stream->cc.zrtp_key);
 	ZSTR_SET_EMPTY(new_stream->cc.peer_zrtp_key);
-
-	ZSTR_SET_EMPTY(new_stream->messages.signaling_hash);
 
 	new_stream->dh_cc.initialized_with	= ZRTP_COMP_UNKN;
 	bnBegin(&new_stream->dh_cc.peer_pv);
@@ -466,6 +464,7 @@ zrtp_status_t zrtp_stream_attach(zrtp_session_t *session, zrtp_stream_t** stream
 
 	zrtp_memset(&new_stream->messages, 0, sizeof(new_stream->messages));	
 	ZSTR_SET_EMPTY(new_stream->messages.h0);
+	ZSTR_SET_EMPTY(new_stream->messages.signaling_hash);
 
 	/* Generate Random nonce, compute H1 and store in the DH packet */
 	new_stream->messages.h0.length = (uint16_t)zrtp_randstr( new_stream->zrtp,
@@ -595,11 +594,11 @@ zrtp_status_t zrtp_signaling_hash_get( zrtp_stream_t* stream,
 	zrtp_string32_t hash_str = ZSTR_INIT_EMPTY(hash_str);
 	zrtp_hash_t *hash = NULL;
 
-	if (!stream) {
+	if (!stream || !hash_buff) {
 		return zrtp_status_bad_param;
 	}
 
-	if (ZRTP_MESSAGE_HASH_SIZE*2+1 > hash_buff_length) {
+	if (ZRTP_SIGN_ZRTP_HASH_LENGTH > hash_buff_length) {
 		return zrtp_status_buffer_size;
 	}
 
@@ -622,29 +621,26 @@ zrtp_status_t zrtp_signaling_hash_set( zrtp_stream_t* ctx,
 									   const char *hash_buff,
 									   uint32_t hash_buff_length)
 {
-	if (!ctx) {
+	if (!ctx || !hash_buff) {
 		return zrtp_status_bad_param;
 	}
 
-	if (ZRTP_MESSAGE_HASH_SIZE*2 < hash_buff_length) {
+	if (ZRTP_SIGN_ZRTP_HASH_LENGTH > hash_buff_length) {
 		return zrtp_status_buffer_size;
 	}
 
 	if (ctx->state != ZRTP_STATE_ACTIVE) {
 		return zrtp_status_wrong_state;
 	}
 
-	str2hex( hash_buff,
-		     hash_buff_length,
-			 ctx->messages.signaling_hash.buffer,
-			 ctx->messages.signaling_hash.max_length);
+	str2hex(hash_buff,
+			ZRTP_SIGN_ZRTP_HASH_LENGTH,
+			ctx->messages.signaling_hash.buffer,
+			ctx->messages.signaling_hash.max_length);
 	ctx->messages.signaling_hash.length = ZRTP_MESSAGE_HASH_SIZE;
 
-	{
-	char buff[64];
-	ZRTP_LOG(3, (_ZTU_,"SIGNALLING HAS was ADDED for the comparision. ID=%u\n", ctx->id));
-	ZRTP_LOG(3, (_ZTU_,"Hash=%s.\n", hex2str(hash_buff, hash_buff_length, buff, sizeof(buff))));
-	}
+	ZRTP_LOG(3, (_ZTU_,"SIGNALLING HAS was ADDED for the comparison. ID=%u\n", ctx->id));
+	ZRTP_LOG(3, (_ZTU_,"Hash=%.*s.\n", ZRTP_SIGN_ZRTP_HASH_LENGTH, hash_buff));
 
 	return zrtp_status_ok;
 }

diff --git a/libs/libzrtp/src/zrtp_string.c b/libs/libzrtp/src/zrtp_string.c
@@ -113,15 +113,16 @@ const char* hex2str(const char* bin, int bin_size, char* buff, int buff_size)
 	if (NULL == buff) {
 		return "buffer is NULL";
 	}		
-	if (buff_size < bin_size*2+1) {
+	if (buff_size < bin_size*2) {
 		return "buffer too small";
 	}
 
 	while (bin_size--) {
 		nptr = hex2char(nptr, *bin++);
 	}
 
-	*nptr = 0;
+	if (buff_size >= bin_size*2+1)
+		*nptr = 0;
 
 	return buff;
 }