CVE-2014-6417: Resource Management Errors (CWE-399)
value: net/ceph/auth_x.c in Ceph, as used in the Linux kernel before 3.16.3, does not properly consider the possibility of kmalloc failure, which allows remote attackers to cause a denial of service (system crash) or possibly have unspecified other impact via a long unencrypted auth ticket.
CWE description: Weaknesses in this category are related to improper management of system resources.
Published: 2014-09-28T10:55Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: net/ceph/auth_x.c
Commit date: 2014-09-10 20:08:36 +0400
Commit message: libceph: do not hard code max auth ticket len

We hard code cephx auth ticket buffer size to 256 bytes.  This isn't
enough for any moderate setups and, in case tickets themselves are not
encrypted, leads to buffer overflows (ceph_x_decrypt() errors out, but
ceph_decode_copy() doesn't - it's just a memcpy() wrapper).  Since the
buffer is allocated dynamically anyway, allocated it a bit later, at
the point where we know how much is going to be needed.

Fixes: http://tracker.ceph.com/issues/8979

Cc: stable@vger.kernel.org
Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Reviewed-by: Sage Weil <sage@redhat.com>

Diff: diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
index 0eb146dce1aa9e..de6662b14e1f5d 100644
--- a/net/ceph/auth_x.c
+++ b/net/ceph/auth_x.c
@@ -11,12 +11,10 @@
 
 #include "crypto.h"
 #include "auth_x.h"
 #include "auth_x_protocol.h"
 
-#define TEMP_TICKET_BUF_LEN	256
-
 static void ceph_x_validate_tickets(struct ceph_auth_client *ac, int *pneed);
 
 static int ceph_x_is_authenticated(struct ceph_auth_client *ac)
 {
 	struct ceph_x_info *xi = ac->private;
@@ -62,23 +60,29 @@ static int ceph_x_encrypt(struct ceph_crypto_key *secret,
 	ceph_encode_32(&obuf, len);
 	return len + sizeof(u32);
 }
 
 static int ceph_x_decrypt(struct ceph_crypto_key *secret,
-			  void **p, void *end, void *obuf, size_t olen)
+			  void **p, void *end, void **obuf, size_t olen)
 {
 	struct ceph_x_encrypt_header head;
 	size_t head_len = sizeof(head);
 	int len, ret;
 
 	len = ceph_decode_32(p);
 	if (*p + len > end)
 		return -EINVAL;
 
 	dout("ceph_x_decrypt len %d\n", len);
-	ret = ceph_decrypt2(secret, &head, &head_len, obuf, &olen,
-			    *p, len);
+	if (*obuf == NULL) {
+		*obuf = kmalloc(len, GFP_NOFS);
+		if (!*obuf)
+			return -ENOMEM;
+		olen = len;
+	}
+
+	ret = ceph_decrypt2(secret, &head, &head_len, *obuf, &olen, *p, len);
 	if (ret)
 		return ret;
 	if (head.struct_v != 1 || le64_to_cpu(head.magic) != CEPHX_ENC_MAGIC)
 		return -EPERM;
 	*p += len;
@@ -129,22 +133,23 @@ static void remove_ticket_handler(struct ceph_auth_client *ac,
 	kfree(th);
 }
 
 static int process_one_ticket(struct ceph_auth_client *ac,
 			      struct ceph_crypto_key *secret,
-			      void **p, void *end,
-			      void *dbuf, void *ticket_buf)
+			      void **p, void *end)
 {
 	struct ceph_x_info *xi = ac->private;
 	int type;
 	u8 tkt_struct_v, blob_struct_v;
 	struct ceph_x_ticket_handler *th;
+	void *dbuf = NULL;
 	void *dp, *dend;
 	int dlen;
 	char is_enc;
 	struct timespec validity;
 	struct ceph_crypto_key old_key;
+	void *ticket_buf = NULL;
 	void *tp, *tpend;
 	struct ceph_timespec new_validity;
 	struct ceph_crypto_key new_session_key;
 	struct ceph_buffer *new_ticket_blob;
 	unsigned long new_expires, new_renew_after;
@@ -165,12 +170,11 @@ static int process_one_ticket(struct ceph_auth_client *ac,
 		ret = PTR_ERR(th);
 		goto out;
 	}
 
 	/* blob for me */
-	dlen = ceph_x_decrypt(secret, p, end, dbuf,
-			      TEMP_TICKET_BUF_LEN);
+	dlen = ceph_x_decrypt(secret, p, end, &dbuf, 0);
 	if (dlen <= 0) {
 		ret = dlen;
 		goto out;
 	}
 	dout(" decrypted %d bytes\n", dlen);
@@ -193,24 +197,29 @@ static int process_one_ticket(struct ceph_auth_client *ac,
 	dout(" expires=%lu renew_after=%lu\n", new_expires,
 	     new_renew_after);
 
 	/* ticket blob for service */
 	ceph_decode_8_safe(p, end, is_enc, bad);
-	tp = ticket_buf;
 	if (is_enc) {
 		/* encrypted */
 		dout(" encrypted ticket\n");
-		dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf,
-				      TEMP_TICKET_BUF_LEN);
+		dlen = ceph_x_decrypt(&old_key, p, end, &ticket_buf, 0);
 		if (dlen < 0) {
 			ret = dlen;
 			goto out;
 		}
+		tp = ticket_buf;
 		dlen = ceph_decode_32(&tp);
 	} else {
 		/* unencrypted */
 		ceph_decode_32_safe(p, end, dlen, bad);
+		ticket_buf = kmalloc(dlen, GFP_NOFS);
+		if (!ticket_buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		tp = ticket_buf;
 		ceph_decode_need(p, end, dlen, bad);
 		ceph_decode_copy(p, ticket_buf, dlen);
 	}
 	tpend = tp + dlen;
 	dout(" ticket blob is %d bytes\n", dlen);
@@ -235,10 +244,12 @@ static int process_one_ticket(struct ceph_auth_client *ac,
 	     type, ceph_entity_type_name(type), th->secret_id,
 	     (int)th->ticket_blob->vec.iov_len);
 	xi->have_keys |= th->service;
 
 out:
+	kfree(ticket_buf);
+	kfree(dbuf);
 	return ret;
 
 bad:
 	ret = -EINVAL;
 	goto out;
@@ -247,49 +258,31 @@ static int process_one_ticket(struct ceph_auth_client *ac,
 static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
 				    struct ceph_crypto_key *secret,
 				    void *buf, void *end)
 {
 	void *p = buf;
-	char *dbuf;
-	char *ticket_buf;
 	u8 reply_struct_v;
 	u32 num;
 	int ret;
 
-	dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
-	if (!dbuf)
-		return -ENOMEM;
-
-	ret = -ENOMEM;
-	ticket_buf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
-	if (!ticket_buf)
-		goto out_dbuf;
-
 	ceph_decode_8_safe(&p, end, reply_struct_v, bad);
 	if (reply_struct_v != 1)
 		return -EINVAL;
 
 	ceph_decode_32_safe(&p, end, num, bad);
 	dout("%d tickets\n", num);
 
 	while (num--) {
-		ret = process_one_ticket(ac, secret, &p, end,
-					 dbuf, ticket_buf);
+		ret = process_one_ticket(ac, secret, &p, end);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
-	ret = 0;
-out:
-	kfree(ticket_buf);
-out_dbuf:
-	kfree(dbuf);
-	return ret;
+	return 0;
 
 bad:
-	ret = -EINVAL;
-	goto out;
+	return -EINVAL;
 }
 
 static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
 				   struct ceph_x_ticket_handler *th,
 				   struct ceph_x_authorizer *au)
@@ -601,17 +594,18 @@ static int ceph_x_verify_authorizer_reply(struct ceph_auth_client *ac,
 {
 	struct ceph_x_authorizer *au = (void *)a;
 	struct ceph_x_ticket_handler *th;
 	int ret = 0;
 	struct ceph_x_authorize_reply reply;
+	void *preply = &reply;
 	void *p = au->reply_buf;
 	void *end = p + sizeof(au->reply_buf);
 
 	th = get_ticket_handler(ac, au->service);
 	if (IS_ERR(th))
 		return PTR_ERR(th);
-	ret = ceph_x_decrypt(&th->session_key, &p, end, &reply, sizeof(reply));
+	ret = ceph_x_decrypt(&th->session_key, &p, end, &preply, sizeof(reply));
 	if (ret < 0)
 		return ret;
 	if (ret != sizeof(reply))
 		return -EPERM;
 

Vulnerable code: #define TEMP_TICKET_BUF_LEN	256

			  void **p, void *end, void *obuf, size_t olen)
	ret = ceph_decrypt2(secret, &head, &head_len, obuf, &olen,
			    *p, len);
			      void **p, void *end,
			      void *dbuf, void *ticket_buf)
	dlen = ceph_x_decrypt(secret, p, end, dbuf,
			      TEMP_TICKET_BUF_LEN);
	tp = ticket_buf;
		dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf,
				      TEMP_TICKET_BUF_LEN);
	char *dbuf;
	char *ticket_buf;
	dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
	if (!dbuf)
		return -ENOMEM;

	ret = -ENOMEM;
	ticket_buf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
	if (!ticket_buf)
		goto out_dbuf;

		ret = process_one_ticket(ac, secret, &p, end,
					 dbuf, ticket_buf);
			goto out;
	ret = 0;
out:
	kfree(ticket_buf);
out_dbuf:
	kfree(dbuf);
	return ret;
	ret = -EINVAL;
	goto out;
	ret = ceph_x_decrypt(&th->session_key, &p, end, &reply, sizeof(reply));
Fixed code: 			  void **p, void *end, void **obuf, size_t olen)
	if (*obuf == NULL) {
		*obuf = kmalloc(len, GFP_NOFS);
		if (!*obuf)
			return -ENOMEM;
		olen = len;
	}

	ret = ceph_decrypt2(secret, &head, &head_len, *obuf, &olen, *p, len);
			      void **p, void *end)
	void *dbuf = NULL;
	void *ticket_buf = NULL;
	dlen = ceph_x_decrypt(secret, p, end, &dbuf, 0);
		dlen = ceph_x_decrypt(&old_key, p, end, &ticket_buf, 0);
		tp = ticket_buf;
		ticket_buf = kmalloc(dlen, GFP_NOFS);
		if (!ticket_buf) {
			ret = -ENOMEM;
			goto out;
		}
		tp = ticket_buf;
	kfree(ticket_buf);
	kfree(dbuf);
		ret = process_one_ticket(ac, secret, &p, end);
			return ret;
	return 0;
	return -EINVAL;
	void *preply = &reply;
	ret = ceph_x_decrypt(&th->session_key, &p, end, &preply, sizeof(reply));