[PATCH v3] erofs-utils: fsck: refuse illegel filename

[Gao Xiang]

On 2023/9/5 10:32, 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")
> Reviewed-by: Guo Xuenan <guoxuenan at huawei.com>
> Signed-off-by: Guo Xuenan <guoxuenan at huawei.com>
> Signed-off-by: Gao Xiang <hsiangkao at linux.alibaba.com>
> ---

changes since v2:
  - should treat '\0' as the end of filename too.

>   lib/dir.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/lib/dir.c b/lib/dir.c
> index fff0bc0..1223cbc 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 && *name != '\0') {
> +		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) {