- djm@cvs.openbsd.org 2010/08/31 09:58:37

     [auth-options.c auth1.c auth2.c bufaux.c buffer.h kex.c key.c packet.c]
     [packet.h ssh-dss.c ssh-rsa.c]
     Add buffer_get_cstring() and related functions that verify that the
     string extracted from the buffer contains no embedded \0 characters*
     This prevents random (possibly malicious) crap from being appended to
     strings where it would not be noticed if the string is used with
     a string(3) function.

     Use the new API in a few sensitive places.

     * actually, we allow a single one at the end of the string for now because
     we don't know how many deployed implementations get this wrong, but don't
     count on this to remain indefinitely.

1 files changed, 4 insertions, 9 deletions

diff --git a/key.c b/key.c
index e4aa25c0..aed4678c 100644
--- a/key.c
+++ b/key.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: key.c,v 1.90 2010/07/13 23:13:16 djm Exp $ */
+/* $OpenBSD: key.c,v 1.91 2010/08/31 09:58:37 djm Exp $ */
 /*
  * read_bignum():
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1067,7 +1067,7 @@ cert_parse(Buffer *b, Key *key, const u_char *blob, u_int blen)
 	principals = exts = critical = sig_key = sig = NULL;
 	if ((!v00 && buffer_get_int64_ret(&key->cert->serial, b) != 0) ||
 	    buffer_get_int_ret(&key->cert->type, b) != 0 ||
-	    (key->cert->key_id = buffer_get_string_ret(b, &kidlen)) == NULL ||
+	    (key->cert->key_id = buffer_get_cstring_ret(b, &kidlen)) == NULL ||
 	    (principals = buffer_get_string_ret(b, &plen)) == NULL ||
 	    buffer_get_int64_ret(&key->cert->valid_after, b) != 0 ||
 	    buffer_get_int64_ret(&key->cert->valid_before, b) != 0 ||
@@ -1105,15 +1105,10 @@ cert_parse(Buffer *b, Key *key, const u_char *blob, u_int blen)
 			error("%s: Too many principals", __func__);
 			goto out;
 		}
-		if ((principal = buffer_get_string_ret(&tmp, &plen)) == NULL) {
+		if ((principal = buffer_get_cstring_ret(&tmp, &plen)) == NULL) {
 			error("%s: Principals data invalid", __func__);
 			goto out;
 		}
-		if (strlen(principal) != plen) {
-			error("%s: Principal contains \\0 character",
-			    __func__);
-			goto out;
-		}
 		key->cert->principals = xrealloc(key->cert->principals,
 		    key->cert->nprincipals + 1, sizeof(*key->cert->principals));
 		key->cert->principals[key->cert->nprincipals++] = principal;
@@ -1200,7 +1195,7 @@ key_from_blob(const u_char *blob, u_int blen)
 #endif
 	buffer_init(&b);
 	buffer_append(&b, blob, blen);
-	if ((ktype = buffer_get_string_ret(&b, NULL)) == NULL) {
+	if ((ktype = buffer_get_cstring_ret(&b, NULL)) == NULL) {
 		error("key_from_blob: can't read key type");
 		goto out;
 	}