Skip to contentSkip to conversation

Patch "KEYS: encrypted: fix buffer overread in valid_master_desc()" has been added to the 3.18-stable tree

PATCH [email protected] 1 participant 1 message Feb 7, 2018

Subsystems (4)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

KEYS: encrypted: fix buffer overread in valid_master_desc()

to the 3.18-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
keys-encrypted-fix-buffer-overread-in-valid_master_desc.patch
and it can be found in the queue-3.18 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 794b4bc292f5d31739d89c0202c54e7dc9bc3add Mon Sep 17 00:00:00 2001

From: Eric Biggers <[email protected]>
Date: Thu, 8 Jun 2017 14:48:18 +0100
Subject: KEYS: encrypted: fix buffer overread in valid_master_desc()

From: Eric Biggers <[email protected]>

commit 794b4bc292f5d31739d89c0202c54e7dc9bc3add upstream.

With the 'encrypted' key type it was possible for userspace to provide a data blob ending with a master key description shorter than expected, e.g. 'keyctl add encrypted desc "new x" @s'. When validating such a master key description, validate_master_desc() could read beyond the end of the buffer. Fix this by using strncmp() instead of memcmp(). [Also clean up the code to deduplicate some logic.]

---
 security/keys/encrypted-keys/encrypted.c |   31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)
security/keys/encrypted-keys/encrypted.c +15 -16
@@ -141,23 +141,22 @@ static int valid_ecryptfs_desc(const cha
  */
 static int valid_master_desc(const char *new_desc, const char *orig_desc)
 {
-	if (!memcmp(new_desc, KEY_TRUSTED_PREFIX, KEY_TRUSTED_PREFIX_LEN)) {
-		if (strlen(new_desc) == KEY_TRUSTED_PREFIX_LEN)
-			goto out;
-		if (orig_desc)
-			if (memcmp(new_desc, orig_desc, KEY_TRUSTED_PREFIX_LEN))
-				goto out;
-	} else if (!memcmp(new_desc, KEY_USER_PREFIX, KEY_USER_PREFIX_LEN)) {
-		if (strlen(new_desc) == KEY_USER_PREFIX_LEN)
-			goto out;
-		if (orig_desc)
-			if (memcmp(new_desc, orig_desc, KEY_USER_PREFIX_LEN))
-				goto out;
-	} else
-		goto out;
+	int prefix_len;
+
+	if (!strncmp(new_desc, KEY_TRUSTED_PREFIX, KEY_TRUSTED_PREFIX_LEN))
+		prefix_len = KEY_TRUSTED_PREFIX_LEN;
+	else if (!strncmp(new_desc, KEY_USER_PREFIX, KEY_USER_PREFIX_LEN))
+		prefix_len = KEY_USER_PREFIX_LEN;
+	else
+		return -EINVAL;
+
+	if (!new_desc[prefix_len])
+		return -EINVAL;
+
+	if (orig_desc && strncmp(new_desc, orig_desc, prefix_len))
+		return -EINVAL;
+
 	return 0;
-out:
-	return -EINVAL;
 }
 
 /*

Patches currently in stable-queue which might be from [email protected] are

queue-3.18/keys-encrypted-fix-buffer-overread-in-valid_master_desc.patch