archive/squashfs: don't crash on malformed squashfs images GHSA-6jcg-q3wp-x2f4 CVE-PENDING

The archive backend passes remote .sqfs bytes straight to the go-diskfs squashfs parser, which does not validate several attacker-controlled superblock and metadata fields. A crafted image can make the parser panic. Recover panics at the go-diskfs boundary and return an "invalid or corrupt squashfs image" error instead. As well as the parse entry points (Read, ReadDir, OpenFile) this wraps the reader returned by Open, since the parser reads file data lazily and can panic long after the image opened successfully. Addresses GHSA-6jcg-q3wp-x2f4.
2026-07-31 13:21:59 +01:00
parent b7a1184019
commit 31f3856281
3 changed files with 145 additions and 3 deletions
@@ -43,9 +43,32 @@ type Fs struct {
 	root        string   // position to read from within the archive
 }

+// recoverParsePanic converts a panic raised while parsing a squashfs image
+// into an error assigned through err.
+func recoverParsePanic(err *error) {
+	if r := recover(); r != nil {
+		*err = fmt.Errorf("invalid or corrupt squashfs image: %v", r)
+	}
+}
+
+// recoveringReadCloser wraps a reader returned by the go-diskfs parser so
+// panics raised while reading become errors.
+//
+// The parser reads file data lazily, so an image which opens cleanly can
+// still panic on the first Read.
+type recoveringReadCloser struct {
+	io.ReadCloser
+}
+
+func (r recoveringReadCloser) Read(p []byte) (n int, err error) {
+	defer recoverParsePanic(&err)
+	return r.ReadCloser.Read(p)
+}
+
 // New constructs an Fs from the (wrappedFs, remote) with the objects
 // prefix with prefix and rooted at root
-func New(ctx context.Context, wrappedFs fs.Fs, remote, prefix, root string) (fs.Fs, error) {
+func New(ctx context.Context, wrappedFs fs.Fs, remote, prefix, root string) (_ fs.Fs, err error) {
+	defer recoverParsePanic(&err)
 	// FIXME vfs cache?
 	// FIXME could factor out ReadFileHandle and just use that rather than the full VFS
 	fs.Debugf(nil, "Squashfs: New: remote=%q, prefix=%q, root=%q", remote, prefix, root)
@@ -201,6 +224,7 @@ func (f *Fs) objectFromFileInfo(nativeDir string, item os.FileInfo) *Object {
 // found.
 func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err error) {
 	defer log.Trace(f, "dir=%q", dir)("entries=%v, err=%v", &entries, &err)
+	defer recoverParsePanic(&err)

 	nativeDir, err := f.toNative(dir)
 	if err != nil {
@@ -242,6 +266,7 @@ func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err e

 // newObjectNative finds the object at the native path passed in
 func (f *Fs) newObjectNative(nativePath string) (o fs.Object, err error) {
+	defer recoverParsePanic(&err)
 	// get the path and filename
 	dir, leaf := path.Split(nativePath)
 	dir = strings.TrimRight(dir, "/")
@@ -394,6 +419,7 @@ func (o *Object) Hash(ctx context.Context, ht hash.Type) (string, error) {

 // Open opens the file for read.  Call Close() on the returned io.ReadCloser
 func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (rc io.ReadCloser, err error) {
+	defer recoverParsePanic(&err)
 	var offset, limit int64 = 0, -1
 	for _, option := range options {
 		switch x := option.(type) {
@@ -426,13 +452,15 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (rc io.Read
 			return nil, err
 		}
 	}
+	rc = recoveringReadCloser{fh}
+
 	// If limited then don't return everything
 	if limit >= 0 {
 		fs.Debugf(nil, "limit=%d, offset=%d, options=%v", limit, offset, options)
-		return readers.NewLimitedReadCloser(fh, limit), nil
+		return readers.NewLimitedReadCloser(rc, limit), nil
 	}

-	return fh, nil
+	return rc, nil
 }

 // Update in to the object with the modTime given of the given size
@@ -0,0 +1,25 @@
+package squashfs
+
+import (
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// panickingReader panics on Read, standing in for the go-diskfs parser
+// hitting an unchecked slice while reading a malformed image.
+type panickingReader struct{}
+
+func (panickingReader) Read([]byte) (int, error) { panic("boom") }
+func (panickingReader) Close() error             { return nil }
+
+// Test that a panic raised while reading file data becomes an error. The
+// parser reads lazily, so an image can open cleanly and only panic here.
+func TestRecoveringReadCloser(t *testing.T) {
+	var rc io.ReadCloser = recoveringReadCloser{panickingReader{}}
+
+	_, err := rc.Read(make([]byte, 16))
+	assert.ErrorContains(t, err, "invalid or corrupt squashfs image")
+	assert.ErrorContains(t, err, "boom")
+}
@@ -0,0 +1,89 @@
+//go:build !plan9
+
+package archive
+
+import (
+	"context"
+	"encoding/binary"
+	"os"
+	"path/filepath"
+	"testing"
+
+	_ "github.com/rclone/rclone/backend/local"
+	"github.com/rclone/rclone/fs/cache"
+	"github.com/rclone/rclone/fstest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// squashfsSuperblock builds a minimal 96-byte squashfs superblock with the
+// given attacker-controllable fields. It mirrors the on-disk layout the
+// go-diskfs parser reads, and is only used to construct deliberately malformed
+// images for the tests below.
+func squashfsSuperblock(blockSize uint32, blockLog, compression uint16, rootInode, inodeTableStart uint64) []byte {
+	const u64max = ^uint64(0)
+	b := make([]byte, 96)
+	binary.LittleEndian.PutUint32(b[0:], 0x73717368) // "hsqs" magic
+	binary.LittleEndian.PutUint32(b[4:], 1)          // inode count
+	binary.LittleEndian.PutUint32(b[8:], 0)          // modtime
+	binary.LittleEndian.PutUint32(b[12:], blockSize)
+	binary.LittleEndian.PutUint32(b[16:], 0) // fragment count
+	binary.LittleEndian.PutUint16(b[20:], compression)
+	binary.LittleEndian.PutUint16(b[22:], blockLog)
+	binary.LittleEndian.PutUint16(b[24:], 0x0210) // no fragments/xattrs
+	binary.LittleEndian.PutUint16(b[26:], 0)      // id count
+	binary.LittleEndian.PutUint16(b[28:], 4)      // version major
+	binary.LittleEndian.PutUint16(b[30:], 0)      // version minor
+	binary.LittleEndian.PutUint64(b[32:], rootInode)
+	binary.LittleEndian.PutUint64(b[40:], 96) // bytes used
+	binary.LittleEndian.PutUint64(b[48:], u64max)
+	binary.LittleEndian.PutUint64(b[56:], u64max)
+	binary.LittleEndian.PutUint64(b[64:], inodeTableStart)
+	binary.LittleEndian.PutUint64(b[72:], inodeTableStart)
+	binary.LittleEndian.PutUint64(b[80:], u64max)
+	binary.LittleEndian.PutUint64(b[88:], u64max)
+	return b
+}
+
+// TestArchiveSquashfsMalformed feeds the archive backend squashfs images whose
+// superblock/metadata make the go-diskfs parser panic. See GHSA-6jcg-q3wp-x2f4.
+func TestArchiveSquashfsMalformed(t *testing.T) {
+	fstest.Initialise()
+	ctx := context.Background()
+
+	// Variant 1: zero block size reaches an integer division in the parser.
+	zeroBlock := squashfsSuperblock(0, 0, 1, 0, 96)
+
+	// Variant 2: an out-of-range root inode byte offset reaches an unchecked
+	// slice against an empty, uncompressed metadata block.
+	oob := append(squashfsSuperblock(65536, 16, 1, 12336, 96), 0x00, 0x80)
+	binary.LittleEndian.PutUint64(oob[40:], uint64(len(oob)))
+
+	for _, tc := range []struct {
+		name  string
+		image []byte
+	}{
+		{"ZeroBlockSize", zeroBlock},
+		{"OutOfRangeInodeOffset", oob},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			// A directory per image: the fs cache keys on the parent
+			// directory, so sharing one would hand the second subtest a
+			// cached listing which does not contain its image.
+			path := filepath.Join(t.TempDir(), tc.name+".sqfs")
+			require.NoError(t, os.WriteFile(path, tc.image, 0o644))
+
+			// The whole point is that this returns rather than panicking and
+			// killing the test binary (and, in production, the serve daemon).
+			f, err := cache.Get(ctx, ":archive:"+path)
+			if err == nil {
+				// The image parsed, so the panic must surface from a later
+				// lazily-parsed operation instead.
+				_, err = f.List(ctx, "")
+			}
+			require.Error(t, err, "a malformed squashfs image must error, not panic")
+			assert.Contains(t, err.Error(), "invalid or corrupt squashfs image",
+				"error should come from the recover, not an unrelated failure")
+		})
+	}
+}