libe2p: avoid segfault when s_nr_users is too high

Currently in e2fsprogs tools it's possible to access out of bounds
memory when reading list of ids sharing a journal log
(journal_superblock_t->s_users[]) in case where s_nr_users is too high.

This is because we never check whether the s_nr_users fits into the
restriction of JFS_USERS_MAX. Fix it by checking that nr_users is not
bigger than JFS_USERS_MAX and error out when possiblem.

Also add test for dumpe2fs. The rest would require involving external
journal which is not possible to test with e2fsprogs test suite at the
moment.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/lib/e2p/ljs.c b/lib/e2p/ljs.c
index 0b1bead..c99126b 100644 (file)
--- a/lib/e2p/ljs.c
+++ b/lib/e2p/ljs.c
@@ -101,10 +101,10 @@ void e2p_list_journal_super(FILE *f, char *journal_sb_buf,
                        e2p_be32(jsb->s_checksum));
        if ((nr_users > 1) ||
            !e2p_is_null_uuid(&jsb->s_users[0])) {
-               for (i=0; i < nr_users; i++) {
+               for (i=0; i < nr_users && i < JFS_USERS_MAX; i++) {
                        printf(i ? "                          %s\n"
                               : "Journal users:            %s\n",
-                              e2p_uuid2str(&jsb->s_users[i*16]));
+                              e2p_uuid2str(&jsb->s_users[i * UUID_SIZE]));
                }
        }
        if (jsb->s_errno != 0)

diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c
index 7f78291..a90e80e 100644 (file)
--- a/lib/ext2fs/mkjournal.c
+++ b/lib/ext2fs/mkjournal.c
@@ -401,6 +401,8 @@ errcode_t ext2fs_add_journal_device(ext2_filsys fs, ext2_filsys journal_dev)
 
        /* Check and see if this filesystem has already been added */
        nr_users = ntohl(jsb->s_nr_users);
+       if (nr_users > JFS_USERS_MAX)
+               return EXT2_ET_CORRUPT_JOURNAL_SB;
        for (i=0; i < nr_users; i++) {
                if (memcmp(fs->super->s_uuid,
                           &jsb->s_users[i*16], 16) == 0)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index b8cddfa..ec977b8 100644 (file)
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -292,6 +292,12 @@ static int remove_journal_device(ext2_filsys fs)
        jsb = (journal_superblock_t *) buf;
        /* Find the filesystem UUID */
        nr_users = ntohl(jsb->s_nr_users);
+       if (nr_users > JFS_USERS_MAX) {
+               fprintf(stderr, _("Journal superblock is corrupted, nr_users\n"
+                                "is too high (%d).\n"), nr_users);
+               commit_remove_journal = 1;
+               goto no_valid_journal;
+       }
 
        if (!journal_user(fs->super->s_uuid, jsb->s_users, nr_users)) {
                fputs(_("Filesystem's UUID not found on journal device.\n"),
@@ -2850,6 +2856,11 @@ fs_update_journal_user(struct ext2_super_block *sb, __u8 old_uuid[UUID_SIZE])
        jsb = (journal_superblock_t *) buf;
        /* Find the filesystem UUID */
        nr_users = ntohl(jsb->s_nr_users);
+       if (nr_users > JFS_USERS_MAX) {
+               fprintf(stderr, _("Journal superblock is corrupted, nr_users\n"
+                                "is too high (%d).\n"), nr_users);
+               return EXT2_ET_CORRUPT_JOURNAL_SB;
+       }
 
        j_uuid = journal_user(old_uuid, jsb->s_users, nr_users);
        if (j_uuid == NULL) {

diff --git a/tests/d_corrupt_journal_nr_users/expect b/tests/d_corrupt_journal_nr_users/expect
new file mode 100644 (file)
index 0000000..cdfb49a
--- /dev/null
+++ b/tests/d_corrupt_journal_nr_users/expect
@@ -0,0 +1,99 @@
+Filesystem volume name:   <none>
+Last mounted on:          <not available>
+Filesystem magic number:  0xEF53
+Filesystem revision #:    1 (dynamic)
+Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit flex_bg sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
+Default mount options:    user_xattr acl
+Filesystem state:         clean
+Errors behavior:          Continue
+Filesystem OS type:       Linux
+Inode count:              512
+Block count:              2048
+Reserved block count:     102
+Free blocks:              982
+Free inodes:              501
+First block:              0
+Block size:               4096
+Fragment size:            4096
+Group descriptor size:    64
+Blocks per group:         32768
+Fragments per group:      32768
+Inodes per group:         512
+Inode blocks per group:   32
+Flex block group size:    16
+Mount count:              0
+Check interval:           0 (<none>)
+Reserved blocks uid:      0
+Reserved blocks gid:      0
+First inode:              11
+Inode size:              256
+Required extra isize:     32
+Desired extra isize:      32
+Journal inode:            8
+Default directory hash:   half_md4
+Journal backup:           inode blocks
+Checksum type:            crc32c
+Journal features:         (none)
+Journal size:             4096k
+Journal length:           1024
+Journal sequence:         0x00000001
+Journal start:            0
+Journal number of users:  9999
+Journal users:            <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+                          <none>
+
+
+Group 0: (Blocks 0-2047)
+  Primary superblock at 0, Group descriptors at 1-1
+  Block bitmap at 2 (+2)
+  Inode bitmap at 18 (+18)
+  Inode table at 34-65 (+34)
+  982 free blocks, 501 free inodes, 2 directories, 501 unused inodes
+  Free blocks: 1066-2047
+  Free inodes: 12-512

diff --git a/tests/d_corrupt_journal_nr_users/image.gz b/tests/d_corrupt_journal_nr_users/image.gz
new file mode 100644 (file)
index 0000000..1fc32ed
Binary files /dev/null and b/tests/d_corrupt_journal_nr_users/image.gz differ

diff --git a/tests/d_corrupt_journal_nr_users/name b/tests/d_corrupt_journal_nr_users/name
new file mode 100644 (file)
index 0000000..8b33a27
--- /dev/null
+++ b/tests/d_corrupt_journal_nr_users/name
@@ -0,0 +1 @@
+Journal superblock corrupted, nr_users too high

diff --git a/tests/d_corrupt_journal_nr_users/script b/tests/d_corrupt_journal_nr_users/script
new file mode 100644 (file)
index 0000000..683cd48
--- /dev/null
+++ b/tests/d_corrupt_journal_nr_users/script
@@ -0,0 +1,25 @@
+if ! test -x $DEBUGFS_EXE; then
+       echo "$test_name: $test_description: skipped (no debugfs)"
+       return 0
+fi
+
+IMAGE=$test_dir/image.gz
+EXP=$test_dir/expect
+OUT=$test_name.log
+gunzip < $IMAGE > $TMPFILE
+
+$DUMPE2FS $TMPFILE >> $OUT.new 2>&1
+sed -f $cmd_dir/filter.sed $OUT.new > $OUT
+rm -f $TMPFILE $OUT.new
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+       echo "$test_name: $test_description: ok"
+       touch $test_name.ok
+else
+       echo "$test_name: $test_description: failed"
+       diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+       rm -f $test_name.tmp
+fi

diff --git a/tests/f_bad_local_jnl/image b/tests/f_bad_local_jnl/image
new file mode 100644 (file)
index 0000000..6f2b550
Binary files /dev/null and b/tests/f_bad_local_jnl/image differ