Repository: diskfs/go-diskfs
Pull request #277 (2025-01-17 00:33:51)

fix: xattr table index to split position into decompressed and compressed offset
fixes: diskfs/go-diskfs/issues/276

deitch: Strange that this fails lint on what you did not touch. I will look.
deitch: This is the oddest thing. I am running the same version of golangci-lint (v1.61.0), the same minor versions of go (tried with v1.23.0 and v1.23.1, while CI is running v1.23.4), and yet I do not see that lint complaint.
deitch: OK, now I got it. It only seems to occur on your branch, even though you didn't change that file. I don't know why. Either way, can you put in a nolint comment on it to clear it? Just run `make lint` locally until it clears.
deitch: Now it's happy. Thanks!

Patch:
diff --git a/filesystem/squashfs/squashfs.go b/filesystem/squashfs/squashfs.go
index 71aac05..f41e84c 100644
--- a/filesystem/squashfs/squashfs.go
+++ b/filesystem/squashfs/squashfs.go
@@ -809,6 +809,7 @@ func readXattrsTable(s *superblock, file backend.File, c Compressor) (*xAttrTabl
 	// now load the actual xAttrs data
 	xAttrEnd := binary.LittleEndian.Uint64(b[:8])
 	xAttrData := make([]byte, 0)
+	offsetMap := map[uint32]uint32{0: 0}
 	for i := xAttrStart; i < xAttrEnd; {
 		uncompressed, size, err = fs.readMetaBlock(file, c, int64(i))
 		if err != nil {
@@ -816,15 +817,16 @@ func readXattrsTable(s *superblock, file backend.File, c Compressor) (*xAttrTabl
 		}
 		xAttrData = append(xAttrData, uncompressed...)
 		i += uint64(size)
+		offsetMap[uint32(i-xAttrStart)] = uint32(len(xAttrData))
 	}
 
 	// now have all of the indexes and metadata loaded
 	// need to pass it the offset of the beginning of the id table from the beginning of the disk
-	return parseXattrsTable(xAttrData, bIndex, s.idTableStart, c)
+	return parseXattrsTable(xAttrData, bIndex, offsetMap, c)
 }
 
-//nolint:unparam,unused,revive // this does not use offset or compressor yet, but only because we have not yet added support
-func parseXattrsTable(bUIDXattr, bIndex []byte, offset uint64, c Compressor) (*xAttrTable, error) {
+//nolint:unparam,unused,revive // this does not use compressor yet, but only because we have not yet added support
+func parseXattrsTable(bUIDXattr, bIndex []byte, offsetMap map[uint32]uint32, c Compressor) (*xAttrTable, error) {
 	// create the ID list
 	var (
 		xAttrIDList []*xAttrIndex
@@ -832,7 +834,7 @@ func parseXattrsTable(bUIDXattr, bIndex []byte, offset uint64, c Compressor) (*x
 
 	entrySize := int(xAttrIDEntrySize)
 	for i := 0; i+entrySize <= len(bIndex); i += entrySize {
-		entry, err := parseXAttrIndex(bIndex[i:])
+		entry, err := parseXAttrIndex(bIndex[i:], offsetMap)
 		if err != nil {
 			return nil, fmt.Errorf("error parsing xAttr ID table entry in position %d: %v", i, err)
 		}
diff --git a/filesystem/squashfs/xattr.go b/filesystem/squashfs/xattr.go
index e189421..12e4cc1 100644
--- a/filesystem/squashfs/xattr.go
+++ b/filesystem/squashfs/xattr.go
@@ -18,15 +18,23 @@ type xAttrIndex struct {
 	size  uint32
 }
 
-func parseXAttrIndex(b []byte) (*xAttrIndex, error) {
+func parseXAttrIndex(b []byte, offsetMap map[uint32]uint32) (*xAttrIndex, error) {
 	if len(b) < int(xAttrIDEntrySize) {
 		return nil, fmt.Errorf("cannot parse xAttr Index of size %d less than minimum %d", len(b), xAttrIDEntrySize)
 	}
-	return &xAttrIndex{
-		pos:   binary.LittleEndian.Uint64(b[0:8]),
+
+	offsetKey := binary.LittleEndian.Uint32(b[2:6])
+	offset, ok := offsetMap[offsetKey]
+	if !ok {
+		return nil, fmt.Errorf("cannot parse xAttr Index invalid offset key %d", offsetKey)
+	}
+	toReturn := &xAttrIndex{
+		pos:   uint64(binary.LittleEndian.Uint16(b[0:2])) + uint64(offset),
 		count: binary.LittleEndian.Uint32(b[8:12]),
 		size:  binary.LittleEndian.Uint32(b[12:16]),
-	}, nil
+	}
+
+	return toReturn, nil
 }
 
 type xAttrTable struct {
@@ -39,6 +47,9 @@ func (x *xAttrTable) find(pos int) (map[string]string, error) {
 		return nil, fmt.Errorf("position %d is greater than list size %d", pos, len(x.list))
 	}
 	entry := x.list[pos]
+	if int(entry.pos) >= len(x.data) {
+		return nil, fmt.Errorf("entry position %d is greater than list size %d", entry.pos, len(x.data))
+	}
 	b := x.data[entry.pos:]
 	count := entry.count
 	ptr := 0


Test patch:
diff --git a/filesystem/squashfs/const_internal_test.go b/filesystem/squashfs/const_internal_test.go
index cb74112..008c28d 100644
--- a/filesystem/squashfs/const_internal_test.go
+++ b/filesystem/squashfs/const_internal_test.go
@@ -199,12 +199,12 @@ func testGetFilesystem(f fs.File) (*FileSystem, []byte, error) {
 	return testFs, b, nil
 }
 
-func testGetInodeMetabytes() (inodeBytes []byte, inodesStart uint64, err error) {
+func testGetInodeMetabytes() (inodeBytes []byte, err error) {
 	testFs, b, err := testGetFilesystem(nil)
 	if err != nil {
-		return nil, 0, err
+		return nil, err
 	}
-	return b[testFs.superblock.inodeTableStart+2:], testFs.superblock.inodeTableStart, nil
+	return b[testFs.superblock.inodeTableStart+2:], nil
 }
 
 //nolint:deadcode // we need these references in the future
diff --git a/filesystem/squashfs/inode_internal_test.go b/filesystem/squashfs/inode_internal_test.go
index 75bb1aa..5bef3f9 100644
--- a/filesystem/squashfs/inode_internal_test.go
+++ b/filesystem/squashfs/inode_internal_test.go
@@ -69,7 +69,7 @@ func TestInodeSize(t *testing.T) {
 }
 
 func TestInodeHeader(t *testing.T) {
-	b, _, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -142,7 +142,7 @@ func TestBlockData(t *testing.T) {
 
 func TestBasicDirectory(t *testing.T) {
 	dir := testBasicDirectory
-	b, _, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -203,7 +203,7 @@ func TestExtendedDirectory(t *testing.T) {
 
 func TestBasicFile(t *testing.T) {
 	f := testBasicFile
-	b, _, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -279,7 +279,7 @@ func TestBasicFile(t *testing.T) {
 func TestExtendedFile(t *testing.T) {
 	fd := testExtendedFile
 	f := &fd
-	b, _, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -332,7 +332,7 @@ func TestExtendedFile(t *testing.T) {
 
 func TestBasicSymlink(t *testing.T) {
 	s := testBasicSymlink
-	b, _, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -430,7 +430,7 @@ func TestExtendedIPC(t *testing.T) {
 }
 
 func TestInode(t *testing.T) {
-	b, _, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/filesystem/squashfs/squashfs_internal_test.go b/filesystem/squashfs/squashfs_internal_test.go
index d5109dc..3e16a50 100644
--- a/filesystem/squashfs/squashfs_internal_test.go
+++ b/filesystem/squashfs/squashfs_internal_test.go
@@ -68,7 +68,7 @@ func TestValidateBlocksize(t *testing.T) {
 
 func TestParseXAttrsTable(t *testing.T) {
 	// parseXattrsTable(bUIDXattr, bIndex []byte, offset uint64, c compressor) (*xAttrTable, error) {
-	b, offset, err := testGetInodeMetabytes()
+	b, err := testGetInodeMetabytes()
 	if err != nil {
 		t.Fatalf("error getting metadata bytes: %v", err)
 	}
@@ -81,7 +81,7 @@ func TestParseXAttrsTable(t *testing.T) {
 
 	// entries in the xattr ID table are offset from beginning of disk, not from xattr table
 	//   so need offset of bUIDXattr from beginning of disk to make use of it
-	table, err := parseXattrsTable(bUIDXattr, bIndex, offset+startUID, nil)
+	table, err := parseXattrsTable(bUIDXattr, bIndex, map[uint32]uint32{0: 0}, nil)
 	if err != nil {
 		t.Fatalf("error reading xattrs table: %v", err)
 	}
diff --git a/filesystem/squashfs/xattr_internal_test.go b/filesystem/squashfs/xattr_internal_test.go
index a58b6c9..ba78cd0 100644
--- a/filesystem/squashfs/xattr_internal_test.go
+++ b/filesystem/squashfs/xattr_internal_test.go
@@ -9,23 +9,43 @@ import (
 func TestParseXAttrIndex(t *testing.T) {
 	tests := []struct {
 		b   []byte
+		o   map[uint32]uint32
 		x   *xAttrIndex
 		err error
 	}{
-		{[]byte{0x0, 0x1}, nil, fmt.Errorf("cannot parse xAttr Index of size %d less than minimum %d", 2, xAttrIDEntrySize)},
+		{[]byte{0x0, 0x1}, map[uint32]uint32{0: 0}, nil, fmt.Errorf("cannot parse xAttr Index of size %d less than minimum %d", 2, xAttrIDEntrySize)},
 		{[]byte{
-			0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
-			0x8, 0x9, 0xa, 0xb,
-			0xc, 0xd, 0xe, 0xf,
-			0x10, 0x11, 0x12, 0x13, 0x14},
+			0x0, 0x1, // position in decompressed
+			0x2, 0x3, 0x4, 0x5, // offset of compressed data
+			0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd,
+			0xe, 0xf, 0x10, 0x11, 0x12, 0x13, 0x14},
+			map[uint32]uint32{},
+			nil, fmt.Errorf("cannot parse xAttr Index invalid offset key %d", 84148994)},
+		{[]byte{
+			0x0, 0x1, // position in decompressed
+			0x2, 0x3, 0x4, 0x5, // offset of compressed data
+			0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd,
+			0xe, 0xf, 0x10, 0x11, 0x12, 0x13, 0x14},
+			map[uint32]uint32{84148994: 5},
+			&xAttrIndex{
+				pos:   261,
+				count: 0x0b0a0908,
+				size:  0x0f0e0d0c,
+			}, nil},
+		{[]byte{
+			0x0, 0x1, // position in decompressed
+			0x2, 0x0, 0x0, 0x0, // offset of compressed data
+			0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd,
+			0xe, 0xf, 0x10, 0x11, 0x12, 0x13, 0x14},
+			map[uint32]uint32{2: 0},
 			&xAttrIndex{
-				pos:   0x0706050403020100,
+				pos:   256,
 				count: 0x0b0a0908,
 				size:  0x0f0e0d0c,
 			}, nil},
 	}
 	for i, tt := range tests {
-		x, err := parseXAttrIndex(tt.b)
+		x, err := parseXAttrIndex(tt.b, tt.o)
 		switch {
 		case (err == nil && tt.err != nil) || (err != nil && tt.err == nil) || (err != nil && tt.err != nil && !strings.HasPrefix(err.Error(), tt.err.Error())):
 			t.Errorf("%d: mismatched error, actual then expected", i)