linux-kernel - [PATCH] eCryptfs: Check array bounds for filename characters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111121233258.GA18466@google.com>
Date:	Mon, 21 Nov 2011 15:32:58 -0800
From:	Michael Halcrow <mhalcrow@...gle.com>
To:	torvalds@...ux-foundation.org, tyhicks@...onical.com
Cc:	ecryptfs@...r.kernel.org, linux-kernel@...r.kernel.org,
	thieule@...gle.com
Subject: [PATCH] eCryptfs: Check array bounds for filename characters

Characters with ASCII values greater than the size of
filename_rev_map[] are valid filename
characters. ecryptfs_decode_from_filename() will access kernel memory
beyond that array, and ecryptfs_parse_tag_70_packet() will then
decrypt those characters. The attacker, using the FNEK of the crafted
file, can then re-encrypt the characters to reveal the kernel memory
past the end of the filename_rev_map[] array. I expect low security
impact since this array is statically allocated in the text area, and
the amount of memory past the array that is accessible is limited by
the largest possible ASCII filename character.

This change verifies that the offset into filename_rev_map[] is within
bounds. If any one character is not, then eCryptfs will consider the
filename to not be a valid encrypted filename and will copy it
verbatim rather than try to decode/decrypt it.

Signed-off-by: Mike Halcrow <mhalcrow@...gle.com>


diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 58609bd..d2d2b9e 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -2032,11 +2032,14 @@ out:
  * @dst_size: Set to the size of the decoded string.
  * @src: The encoded set of octets to decode.
  * @src_size: The size of the encoded set of octets to decode.
+ *
+ * Returns zero on success; non-zero otherwise
  */
-static void
+static int
 ecryptfs_decode_from_filename(unsigned char *dst, size_t *dst_size,
 			      const unsigned char *src, size_t src_size)
 {
+	int rc = 0;
 	u8 current_bit_offset = 0;
 	size_t src_byte_offset = 0;
 	size_t dst_byte_offset = 0;
@@ -2052,9 +2055,13 @@ ecryptfs_decode_from_filename(unsigned char *dst, size_t *dst_size,
 		goto out;
 	}
 	while (src_byte_offset < src_size) {
-		unsigned char src_byte =
-				filename_rev_map[(int)src[src_byte_offset]];
-
+		int rev_map_offset = (int)src[src_byte_offset];
+		unsigned char src_byte;
+		if (rev_map_offset > (ARRAY_SIZE(filename_rev_map) - 1)) {
+			rc = -EINVAL;
+			goto out;
+		}
+		src_byte = filename_rev_map[rev_map_offset];
 		switch (current_bit_offset) {
 		case 0:
 			dst[dst_byte_offset] = (src_byte << 2);
@@ -2081,7 +2088,7 @@ ecryptfs_decode_from_filename(unsigned char *dst, size_t *dst_size,
 	}
 	(*dst_size) = dst_byte_offset;
 out:
-	return;
+	return rc;
 }
 
 /**
@@ -2235,8 +2242,14 @@ int ecryptfs_decode_and_decrypt_filename(char **plaintext_name,
 
 		name += ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE;
 		name_size -= ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE;
-		ecryptfs_decode_from_filename(NULL, &decoded_name_size,
-					      name, name_size);
+		rc = ecryptfs_decode_from_filename(NULL, &decoded_name_size,
+						   name, name_size);
+		if (rc) {
+			rc = ecryptfs_copy_filename(plaintext_name,
+						    plaintext_name_size,
+						    orig_name, orig_name_size);
+			goto out;
+		}
 		decoded_name = kmalloc(decoded_name_size, GFP_KERNEL);
 		if (!decoded_name) {
 			printk(KERN_ERR "%s: Out of memory whilst attempting "
@@ -2245,8 +2258,16 @@ int ecryptfs_decode_and_decrypt_filename(char **plaintext_name,
 			rc = -ENOMEM;
 			goto out;
 		}
-		ecryptfs_decode_from_filename(decoded_name, &decoded_name_size,
-					      name, name_size);
+		rc = ecryptfs_decode_from_filename(decoded_name,
+						   &decoded_name_size,
+						   name,
+						   name_size);
+		if (rc) {
+			rc = ecryptfs_copy_filename(plaintext_name,
+						    plaintext_name_size,
+						    orig_name, orig_name_size);
+			goto out_free;
+		}
 		rc = ecryptfs_parse_tag_70_packet(plaintext_name,
 						  plaintext_name_size,
 						  &packet_size,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/