[PATCH v2 RESEND] erofs-utils: fsck: refuse illegel filename

[Guo Xuenan]

Hi Xiang,
On 2023/9/4 17:53, Gao Xiang wrote:
> From: Guo Xuenan <guoxuenan at huawei.com>
>
> 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] https://github.com/plougher/squashfs-tools/issues/72
> [2] https://github.com/plougher/squashfs-tools/commit/79b5a555058eef4e1e7ff220c344d39f8cd09646
> Fixes: 412c8f908132 ("erofs-utils: fsck: add --extract=X support to extract to path X")
> Signed-off-by: Guo Xuenan <guoxuenan at huawei.com>
> Signed-off-by: Gao Xiang <hsiangkao at linux.alibaba.com>
> ---
> v1: https://lore.kernel.org/r/20230531072612.2643983-2-guoxuenan@huawei.com
>
> changes since v1:
>   - check this only if fsck is enabled (extraction and fsck is impacted);
>
>   - don't treat the middle '\0' as an illegel name since such cases are
>     actually reserved and can be handled properly.
>
>   lib/dir.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/lib/dir.c b/lib/dir.c
> index fff0bc0..3731f0c 100644
> --- a/lib/dir.c
> +++ b/lib/dir.c
> @@ -4,6 +4,19 @@
>   #include "erofs/print.h"
>   #include "erofs/dir.h"
>   
> +/* filename should not have a '/' in the name string */
> +static bool erofs_validate_filename(const char *dname, int size)
> +{
> +	char *name = (char *)dname;
> +
> +	while (name - dname < size) {
> +		if (*name == '/')
> +			return false;
> +		++name;
> +	}
> +	return true;
> +}
> +
>   static int traverse_dirents(struct erofs_dir_context *ctx,
>   			    void *dentry_blk, unsigned int lblk,
>   			    unsigned int next_nameoff, unsigned int maxsize,
> @@ -102,6 +115,10 @@ static int traverse_dirents(struct erofs_dir_context *ctx,
>   				}
>   				break;
>   			}
> +		} else if (fsck &&
> +			   !erofs_validate_filename(de_name, de_namelen)) {
> +			errmsg = "corrupted dirent with illegal filename";
> +			goto out;
>   		}
>   		ret = ctx->cb(ctx);
>   		if (ret) {
Reviewed-by： Guo Xuenan <guoxuenan at huawei.com>

Thanks
Xuenan

-- 
Guo Xuenan [OS Kernel Lab]
-----------------------------
Email: guoxuenan at huawei.com