Hi Richard, [email protected] 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 <[email protected]> > --- > 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
