[patch] Squashfs: sanity check information from disk

From: Dan Carpenter
Date: Mon Jul 15 2013 - 12:17:31 EST


We read the size of the name from the disk, but a larger name than
expected would cause memory corruption.

Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
I don't know this code very well, but to me it looks like there is an
off by one bug here as well.

We say:

size = le32_to_cpu(index->size) + 1;

The "+ 1" is presumably for the NUL terminator. Then we do:

index->name[size] = '\0';

That means we are putting a NUL character one space beyond the end of
the array. Presumably the first character of the next thing saved to
the disk is usually zero so that's why we don't notice that we are
reading a extra character when we read "size" number of bytes.

diff --git a/fs/squashfs/namei.c b/fs/squashfs/namei.c
index 7834a51..bc1334c 100644
--- a/fs/squashfs/namei.c
+++ b/fs/squashfs/namei.c
@@ -79,7 +79,8 @@ static int get_dir_index_using_name(struct super_block *sb,
int len)
{
struct squashfs_sb_info *msblk = sb->s_fs_info;
- int i, size, length = 0, err;
+ int i, length = 0, err;
+ unsigned int size;
struct squashfs_dir_index *index;
char *str;

@@ -103,6 +104,10 @@ static int get_dir_index_using_name(struct super_block *sb,


size = le32_to_cpu(index->size) + 1;
+ if (size >= SQUASHFS_NAME_LEN + 1) {
+ err = -EINVAL;
+ break;
+ }

err = squashfs_read_metadata(sb, index->name, &index_start,
&index_offset, size);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/