Fix a buffer overflow in the engineId preferences. Fixes bug 5530.

svn path=/trunk/; revision=35298

1 files changed, 15 insertions, 11 deletions

diff --git a/asn1/snmp/packet-snmp-template.c b/asn1/snmp/packet-snmp-template.c
index 0a5971c983..3c3250dba2 100644
--- a/asn1/snmp/packet-snmp-template.c
+++ b/asn1/snmp/packet-snmp-template.c
@@ -1814,8 +1814,8 @@ snmp_usm_password_to_key_md5(const guint8 *password, guint passwordlen,
 	/*****************************************************/
 	/* Now localize the key with the engineID and pass   */
 	/* through MD5 to produce final key                  */
-	/* May want to ensure that engineLength <= 32,       */
-	/* otherwise need to use a buffer larger than 64     */
+	/* We ignore invalid engineLengths here. More strict */
+	/* checking is done in snmp_users_update_cb.         */
 	/*****************************************************/
 
 	md5_init(&MD);
@@ -1840,7 +1840,7 @@ snmp_usm_password_to_key_sha1(const guint8 *password, guint passwordlen,
 			      guint8 *key)
 {
 	sha1_context     SH;
-	guint8     *cp, password_buf[72];
+	guint8     *cp, password_buf[64];
 	guint32      password_index = 0;
 	guint32      count = 0, i;
 
@@ -1866,15 +1866,14 @@ snmp_usm_password_to_key_sha1(const guint8 *password, guint passwordlen,
 	/*****************************************************/
 	/* Now localize the key with the engineID and pass   */
 	/* through SHA to produce final key                  */
-	/* May want to ensure that engineLength <= 32,       */
-	/* otherwise need to use a buffer larger than 72     */
+	/* We ignore invalid engineLengths here. More strict */
+	/* checking is done in snmp_users_update_cb.         */
 	/*****************************************************/
-	memcpy(password_buf, key, 20);
-	memcpy(password_buf+20, engineID, engineLength);
-	memcpy(password_buf+20+engineLength, key, 20);
 
 	sha1_starts(&SH);
-	sha1_update(&SH, password_buf, 40+engineLength);
+	sha1_update(&SH, key, 20);
+	sha1_update(&SH, engineID, engineLength);
+	sha1_update(&SH, key, 20);
 	sha1_finish(&SH, key);
 	return;
  }
@@ -1951,6 +1950,11 @@ snmp_users_update_cb(void* p _U_, const char** err)
 	for (i=0; i<num_ueas-1; i++) {
 		snmp_ue_assoc_t* u = &(ueas[i]);
 
+		/* RFC 3411 section 5 */
+		if (u->engine.len < 5 || u->engine.len > 32) {
+			g_string_append_printf(es, "Invalid engineId length (%u). Must be between 5 and 32 (10 and 64 hex digits)\n", u->engine.len);
+		}
+
 
 		if ( u->user.userName.len == ue->user.userName.len
 			&& u->engine.len == ue->engine.len ) {
@@ -1958,13 +1962,13 @@ snmp_users_update_cb(void* p _U_, const char** err)
 			if (u->engine.len > 0 && memcmp( u->engine.data,   ue->engine.data,  u->engine.len ) == 0) {
 				if ( memcmp( u->user.userName.data, ue->user.userName.data, ue->user.userName.len ) == 0 ) {
 					/* XXX: make a string for the engineId */
-					g_string_append_printf(es,"duplicate key (userName='%s')\n",ue->user.userName.data);
+					g_string_append_printf(es,"Duplicate key (userName='%s')\n",ue->user.userName.data);
 				}
 			}
 
 			if (u->engine.len == 0) {
 				if ( memcmp( u->user.userName.data, ue->user.userName.data, ue->user.userName.len ) == 0 ) {
-					g_string_append_printf(es,"duplicate key (userName='%s' engineId=NONE)\n",ue->user.userName.data);
+					g_string_append_printf(es,"Duplicate key (userName='%s' engineId=NONE)\n",ue->user.userName.data);
 				}
 			}
 		}