[PATCH 0/2] erofs-utils: fsck.erofs bugfixes

[PATCH 0/2] erofs-utils: fsck.erofs bugfixes

[Guo Xuenan via Linux-erofs]

In crafted erofs image, fsck.erofs file extraction exposed
some bugs.

Guo Xuenan (2):
  erofs-utils: fsck: fix outside destination directory exploit
  erofs-utils: fsck: fix segmentfault for crafted image extract

 lib/decompress.c |  8 ++++++--
 lib/dir.c        | 21 +++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

-- 
2.31.1

[PATCH 1/2] erofs-utils: fsck: fix outside destination directory exploit

[Guo Xuenan via Linux-erofs]

In some crafted erofs image, fsck.erofs may write outside
destination directory, which may be used to do some very
dangerous things.

This commit fix this exploit by checking all directory
entry names. Squashfs also met same situation [1], and
already fixed it here [2].

[1]: https://github.com/plougher/squashfs-tools/issues/72
[2]: https://github.com/plougher/squashfs-tools/commit/79b5a555058eef4e1e7ff220c344d39f8cd09646
Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
---
 lib/dir.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lib/dir.c b/lib/dir.c
index cb8c188..074e7c3 100644
--- a/lib/dir.c
+++ b/lib/dir.c
@@ -4,6 +4,24 @@
 #include "erofs/print.h"
 #include "erofs/dir.h"
 
+/*
+ * Check name for validity, name should not
+ *  - have a "/" anywhere in the name, or
+ *  - be shorter than the expected size
+ */
+static int erofs_check_name(const char *dname, int size)
+{
+	char *name = (char *)dname;
+
+	while (*name != '/' && *name != '\0' && (name - dname < size))
+		name++;
+	if (*name == '/')
+		return false;
+	if ((name - dname) != size)
+		return false;
+	return true;
+}
+
 static int traverse_dirents(struct erofs_dir_context *ctx,
 			    void *dentry_blk, unsigned int lblk,
 			    unsigned int next_nameoff, unsigned int maxsize,
@@ -101,6 +119,9 @@ static int traverse_dirents(struct erofs_dir_context *ctx,
 				}
 				break;
 			}
+		} else if (!erofs_check_name(de_name, de_namelen)) {
+			errmsg = "corrupted dirent with illegal characters";
+			goto out;
 		}
 		ret = ctx->cb(ctx);
 		if (ret) {
-- 
2.31.1

[PATCH 2/2] erofs-utils: fsck: fix segmentfault for crafted image extract

[Guo Xuenan via Linux-erofs]

In crafted erofs image, extract files may lead to fsck.erofs
memory access out of bounds.
Actually, there is already interception in the code, but which only
take effect in debug mode, change it to avoid that.

Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
---
 lib/decompress.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/decompress.c b/lib/decompress.c
index 8d1b25d..59a9ca0 100644
--- a/lib/decompress.c
+++ b/lib/decompress.c
@@ -138,8 +138,12 @@ int z_erofs_decompress(struct z_erofs_decompress_req *rq)
 		if (rq->inputsize > erofs_blksiz())
 			return -EFSCORRUPTED;
 
-		DBG_BUGON(rq->decodedlength > erofs_blksiz());
-		DBG_BUGON(rq->decodedlength < rq->decodedskip);
+		if (rq->decodedlength > erofs_blksiz())
+			return -EFSCORRUPTED;
+
+		if (rq->decodedlength < rq->decodedskip)
+			return -EFSCORRUPTED;
+
 		count = rq->decodedlength - rq->decodedskip;
 		skip = erofs_blkoff(rq->interlaced_offset + rq->decodedskip);
 		rightpart = min(erofs_blksiz() - skip, count);
-- 
2.31.1

Re: [PATCH 2/2] erofs-utils: fsck: fix segmentfault for crafted image extract

[Gao Xiang]

On 2023/5/31 15:26, Guo Xuenan wrote:
> In crafted erofs image, extract files may lead to fsck.erofs
> memory access out of bounds.
> Actually, there is already interception in the code, but which only
> take effect in debug mode, change it to avoid that.
> 
> Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang