On 2011-11-21 15:32:58, Michael Halcrow wrote:
> 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@xxxxxxxxxx>
Linus - This is a pretty low security risk, but I figure that it is
probably best if you apply it directly to your tree. I don't think there
is much need for it to go through my tree. If you do take it in, feel
free to add the following tags:
Cc: <stable@xxxxxxxxxx>
Acked-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx>
BTW, I reviewed and tested this patch before Mike sent it out to the
list.
Tyler
>
> 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,
Attachment:
signature.asc
Description: Digital signature
