Hi Richard,
richard@nod.at wrote on Fri, 12 Jul 2024 10:23:44 +0200:
The squashfs driver blindly follows symlinks, and calls sqfs_size() recursively. So an attacker can create a crafted filesystem and with a deep enough nesting level a stack overflow can be achieved.
Fix by limiting the nesting level to 8.
As this is I believe an arbitrary value, could we define this value somewhere and flag it with a comment as "arbitrary" with some details from the commit log? Right now the value '8' is hardcoded at least in 3 different places. Also, 8 seems rather small, any reason for choosing that? I believe this is easy to cross even in non-evil filesystems and could perhaps be (again, arbitrarily) increased a bit?
Signed-off-by: Richard Weinberger richard@nod.at
fs/squashfs/sqfs.c | 74 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 15 deletions(-)
...
/* Check for symbolic link and inode type sanity */ if (get_unaligned_le16(&dir->inode_type) == SQFS_SYMLINK_TYPE) {
if (++symlinknest == 8) {ret = -ELOOP;goto out;}
...
@@ -1421,9 +1441,14 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len, break; case SQFS_SYMLINK_TYPE: case SQFS_LSYMLINK_TYPE:
if (++symlinknest == 8) {ret = -ELOOP;goto out;}
...
case SQFS_LSYMLINK_TYPE:
if (++symlinknest == 8) {*size = 0;return -ELOOP;}
Thanks, Miquèl