From 78f6a115c92b602daef686ed9278a11b1d9f734a Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sat, 16 Dec 2023 22:42:12 +0000 Subject: [PATCH 1/2] squashfs: fix received X bytes instead of expected minimal 40 on listing When unpacking large squashfs archives you occasionally get this error: received 38 bytes instead of expected minimal 40 This is due to the inode changing type (ie it is actual an extended inode) when read from the directory but the size not being updated. Most of the time there is enough data for the larger inode but not always. --- filesystem/squashfs/const_internal_test.go | 1 + filesystem/squashfs/squashfs.go | 8 +++++ filesystem/squashfs/squashfs_test.go | 35 +++++++++++++++++++ filesystem/squashfs/testdata/buildtestsqs.sh | 26 ++++++++++++++ filesystem/squashfs/testdata/dir_read.sqs | Bin 0 -> 4096 bytes 5 files changed, 70 insertions(+) create mode 100644 filesystem/squashfs/testdata/dir_read.sqs diff --git a/filesystem/squashfs/const_internal_test.go b/filesystem/squashfs/const_internal_test.go index 7b594500..86012571 100644 --- a/filesystem/squashfs/const_internal_test.go +++ b/filesystem/squashfs/const_internal_test.go @@ -19,6 +19,7 @@ const ( SquashfsfileListing = "./testdata/list.txt" SquashfsReadTestFile = "./testdata/read_test.sqs" SquashfsReatTestMd5sums = "./testdata/read_test.md5sums" + SquashfsReadDirTestFile = "./testdata/dir_read.sqs" ) // first header diff --git a/filesystem/squashfs/squashfs.go b/filesystem/squashfs/squashfs.go index fbfb3c0b..11753dad 100644 --- a/filesystem/squashfs/squashfs.go +++ b/filesystem/squashfs/squashfs.go @@ -453,6 +453,14 @@ func (fs *FileSystem) getInode(blockOffset uint32, byteOffset uint16, iType inod } if header.inodeType != iType { iType = header.inodeType + size = inodeTypeToSize(iType) + // Read more data if necessary (quite rare) + if size > len(uncompressed) { + uncompressed, err = readMetadata(fs.file, fs.compressor, int64(fs.superblock.inodeTableStart), blockOffset, byteOffset, size) + if err != nil { + return nil, fmt.Errorf("error reading block at position %d: %v", blockOffset, err) + } + } } // now read the body, which may have a variable size body, extra, err := parseInodeBody(uncompressed[inodeHeaderSize:], int(fs.blocksize), iType) diff --git a/filesystem/squashfs/squashfs_test.go b/filesystem/squashfs/squashfs_test.go index 05afdb60..a0ca729d 100644 --- a/filesystem/squashfs/squashfs_test.go +++ b/filesystem/squashfs/squashfs_test.go @@ -692,6 +692,41 @@ func TestSquashfsReadDirXattr(t *testing.T) { } } +// Check a squash file with some corner cases +func TestSquashfsReadDirCornerCases(t *testing.T) { + // Open the squash file + f, err := os.Open(squashfs.SquashfsReadDirTestFile) + if err != nil { + t.Fatal(err) + } + defer f.Close() + fi, err := f.Stat() + if err != nil { + t.Fatal(err) + } + // create the filesystem + fs, err := squashfs.Read(f, fi.Size(), 0, 0) + if err != nil { + t.Fatal(err) + } + + fis, err := fs.ReadDir("/") + if err != nil { + t.Fatal(err) + } + want := 300 + if want != len(fis) { + t.Errorf("Want %d entries but got %d", want, len(fis)) + } + for i, fi := range fis { + want := fmt.Sprintf("file_%03d", i+1) + got := fi.Name() + if want != got { + t.Errorf("Want name %q but got %q", want, got) + } + } +} + //nolint:unused,revive // keep for future when we implement it and will need t func TestFinalize(t *testing.T) { diff --git a/filesystem/squashfs/testdata/buildtestsqs.sh b/filesystem/squashfs/testdata/buildtestsqs.sh index cd1eaee1..929e39eb 100755 --- a/filesystem/squashfs/testdata/buildtestsqs.sh +++ b/filesystem/squashfs/testdata/buildtestsqs.sh @@ -73,3 +73,29 @@ mksquashfs . /data/read_test.sqs find . -type f -exec sh -c 'md=$(md5sum "$0"); size=$(wc -c <"$0"); echo ${md} ${size}' {} \; > /data/read_test.md5sums EOF +rm -f dir_read.sqs + +# Build a test sqs file to test corner cases in directory reading +# +# This fills up a metadata page with 300 extended file inodes in order +# to test the corner cases in the directory reading code when reading +# inodes which are assumed to be standard sized but turn out to be +# extended sized. The inodes are forced to be extended by having xattr +# set on them. +# +# It also tests corner cases in the xattr parsing code! +# +# See: TestSquashfsReadDirCornerCases +cat << "EOF" | docker run -i --rm -v $PWD:/data alpine:3.19 +set -e +apk --update add squashfs-tools coreutils attr +mkdir -p /build +cd /build + +for i in $(seq -w 300); do + touch file_$i + setfattr -n user.test -v $i file_$i +done + +mksquashfs . /data/dir_read.sqs -comp zstd -Xcompression-level 3 -b 4k -all-root +EOF diff --git a/filesystem/squashfs/testdata/dir_read.sqs b/filesystem/squashfs/testdata/dir_read.sqs new file mode 100644 index 0000000000000000000000000000000000000000..109fc8a0bab24c444b830298a20ab5d28213efa6 GIT binary patch literal 4096 zcmeHJi93|v7k|eL2E)uSWH)2qw<45f6teHxrm`f8u~#BxvV>ByCy8tmvZNXNRun2* z%9cHA3CR-0??pY&@B4iJh3|Qv=RNnl_n!MX?>YC}bKL!X{be8kKsJnX1yH~q(E$jc z5e5NZpp*%@2HpL*h}fF~{2BLo-yiwhz6jXwE(WOK_s^>$*DRRFZ0f7Ys zaF&e$$N>@eF?T?sfk0_sP+AxrEj=Bao&nCtz{H4PVn(nqBUw;LG>R3?#)@HM$8fM8 z;NU#K#mUXZ!_CXX$IH(rz>gKcVFhtQg2F-~!lEK#qT*r_;*t_llG0K#(y}sgvhs2Y z@`?%v6%QR$I;5<0SXt$;s*0NG5w)X7)Q@VYYiej|;boIqo;d9Pyd90 zzM+AUp|O#Pv8jofskxbjxuu1brM1;bYnzj{Hg>l5b`JIghf{>pr_P*qJmci(?BwEn z*2VSgIajxH?r!JZFP!(d;ORm1^dfqD`FQ*K`1$(#1^8bKxODOIrNGNq0)wsu2VD&g zxq2<+`nAyOq|h6rup8lF5#f;$HzT8NMn}a&--@|?EB5xC*t>V)?%s=wzn1`dizG(I zZMb%hOYyO4Vd27Xarh?fUDU4?s20}+tbw?K76ZHp&((`HFjCZFKo#K?da*IKidtw? z5lOKZn`o;T%8D`yQ|#4Bv~>oDwqc57d}1o1+iiyQHb(;pdM6m)XIvg%Id0`>e`yR+WNYN`o@N) z#urU5Uo^jLX@1r6`c>=ew$}ExH|=lVynEa6uCt@7v%9OO`+ZOE`wzVzKlFXG&KBaWO#IBY;=5VVtjJq^W@a$>8Y9N*_pZ7`MEFiU%xDT{kE|9ZE10N>HG4^ z_a7^(Kh{>)*EZIFZfySC+WfWk``7mGo$Vb64HQO8M-OLUWI`~rAW>*mHViw*0ZuM% z9$r3v0W3~XNLYkqOkQ3}R_eZKPy=Bd!@-->Z&=~cD8XMbjYwj}hKh<_?x5C5gP0mp zQ6i>-WG^A%1+}xb_(b>2!lH`#Y;H~^U#o$>FHdW7{Dg8`l41)HD<#r0tQEtTTL$bz z<=j1A2-O~HiSWJQ`zpFIRWOo4o69E#XCmLB zg66brmdA@jO%$3LUU7-hWc#k)g%xalG`m^Uc!I*c9RTcQUn^h@1p>fy{2NqZZ~z8% zFZchEJ8)nDvU@4efKi`em4)n{fCGS|qdc5OtBaEcXovDa4Isl15$Ho;igX8(8~QCY zo@7Fuz~*O8#TXxymk|Is9ASH?pV1USpxX2o@_r-G`)l3$$dR8aO_)Y%RP2}_HJpfcaO8G4egpww zg}echUh4NSA8D!@>^}9fqv+t*($6iWY|~6?8lL!>OH#9q(zJX8YqfhOtpmmks?YUW z&nt!VCO${{TpNI}u(+l7x2Q-4`x<>1S#qy?VjMKNS*yQF41Gp5mNn$z@^UP%MtGWB z!W)-JlFPbbN+r>my4#@={SpZ#7SKF9Suo{k@|IXKm+c^jD!4=GbRk`1(T}4|(*~4! z5XK$Z_fIOP;l~BeLm7WJGa@5h<+QDg^_F@Uz@+Ubj59JO#lqf4Qk|)^nO5)*tUTL{}ur zxzARb3-vUo*v2@}P(1EanckbUa8@d|0yjT5!S(v6+q`v~%Rx%z+|wvs!FssT=S!$CFE|f)@&pbOOe| z(8R2@bQV`{WOZ|BK5d(zUuD;5I5{4cpc-K+U2)ZlGve-C#_YHqn>xk*@oOFZA34)- z{xO}U&uJ|}2`aEo$*?iiab*6;s?W0lmV6uet;x-m`h**m^{F>Wt;-(;EHX)g(0ERN1Ml8wdJt zd46+v$DCcY{Fcb}D7dfcyy9i4$zr_Ir-oZRZoaQ2z1UlbipHO6CySN5QkoyOSCBR? zeH%$07%B`3o<8oqK203*ZrG$IR)h^uS%xxB4M}?(5iO#I^sW&HBZrl}hUCo$N2Uf3 z5o#`aG-P@%oOv3XO>rrw&QlEYir*zbEIMpDQdBhQyz4urJ1=P5n!c%!%-9Ui7Jj92 za3!R(;i87o^yKYF7=`U`6Yr(36;23ciEn#POx_XjxSG_gsW=tPUtcmDoKk1{AsAcN z`7U^1(I(_fK>T{0r}xXX5)ZkjA*F*~$iF=Y%e^!))H>6_Q9pG=-=u?pu0uoN>LL5l zv{h|#-%d5Z*cX@krz7UVj8Zb(<~)){bmlA*_yacy@81ajmh9X~U%I;3G4%CUVTYlb zK>A8`XJ=4`$V~8yUt3GpYp#=I$Y^0$S0O*&vf&T}Ee>OBLk6`>h!AB2j37aeC6Rfk zAoC>pe4vh;L~n_$Be#c9x7%oVkf~AGl?l`36k;`>%+I`q+4#f5LHTA7Add8>t8j;d zUB#M>xr~qD_i&~Z{sXdvAVOucu8?-PF0D9*F+w}bOdK;0W1YKlv~MM_cxKs!VhlM3 z=zveLxAXyyzBP%f32$L^G6w{O^|I(?oa5XxGYCwCQQN#0N(gLJ*gq8iCW5BT++FJy zr?5tEf>h`io;R0^W=cl4w~%e-WS2>YW~{<7^(yDe`m$PE!jRjtv7!{`bHa&xyb&hP zqwo_Jrp2rvmJo=A2eIfuEHw~|8N_M@v8*qOl`HgG)VG6ng>7p6t+t`~{ zW2c%X1~tOG)+~Hy2g$5g%+hx_85w(q zFq8sLD1Z}zpb_?+~k|v{3n)Kr( z5HuFviS!Gp0akOmeTx~jRjq~y+>k`E*gO6*@F_FmbPqf;qw*4DzAUXhYY>#M>gNN} zJ@Dc;+m%yv_q1w*6F;o4GiU#83SpsOK>>(=TEn;dWhoKGP6wRB2(y;v+NuVe=j;*{ z6coM~FB$F#JqU;btN@d%j}h*ebX=1{1fooy?lnfS`35JiWh;t_es`=L=EBJ3i~~1@ zVNeJR05+g7>wQvE$&15r>@RSV-xE5mWbxnGV<$|--*J+`>bOv5Xsrf=(ZDIziZ zSyR{Ox8DdTkEH4en{yY#6CRf}c8)CkVuEo?sOX(^4G4?RDsAW({<_6T%O!qT*ZQpg zjeD6T_3u7?*<_&O6jRo*a`7X@Ju0qyJ2d|jPJcjD>A0n{Z|L2Or?qbe=Qil!93qE~ zSvdJzzw@xDrhQ;`osNNB_@K7AqxZGg^up@4{+TseMvRam-t3H5$n6IORjqx~t1u=u zK?N<-)5NQ{(wo=}H_XWHOty!g#tu49A*roYS zUOB3*YVtOtY3+}GGg1G1E~TRzJ!SgbdL4`aZR`2qKW9vOx7PtlpJ|`}_x{fV|9Rm5 GJn%nO(YGQ1 literal 0 HcmV?d00001 From fe4709c922ed644ef4aadb61c2f961f07853ecf3 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sat, 30 Dec 2023 17:15:29 +0000 Subject: [PATCH 2/2] squashfs: fix slice bounds out of range when parsing xattr Before this fix the code crashed with errors like panic: runtime error: slice bounds out of range [282:270] [recovered] When reading xattrs. This was caused by a mixup of indices in the code. This fix is tested by TestSquashfsReadDirCornerCases and causes it to run clean. --- filesystem/squashfs/xattr.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/filesystem/squashfs/xattr.go b/filesystem/squashfs/xattr.go index f63d4029..e1894217 100644 --- a/filesystem/squashfs/xattr.go +++ b/filesystem/squashfs/xattr.go @@ -45,7 +45,7 @@ func (x *xAttrTable) find(pos int) (map[string]string, error) { xattrs := map[string]string{} for i := 0; i < int(count); i++ { // must be 4 bytes for header - if len(b[pos:]) < 4 { + if len(b[ptr:]) < 4 { return nil, fmt.Errorf("insufficient bytes %d to read the xattr at position %d", len(b[ptr:]), ptr) } // get the type and size @@ -56,7 +56,7 @@ func (x *xAttrTable) find(pos int) (map[string]string, error) { valStart := valHeaderStart + 4 // make sure we have enough bytes if len(b[nameStart:]) < xSize { - return nil, fmt.Errorf("xattr header has size %d, but only %d bytes available to read at position %d", xSize, len(b[pos+4:]), ptr) + return nil, fmt.Errorf("xattr header has size %d, but only %d bytes available to read at position %d", xSize, len(b[ptr+4:]), ptr) } if xSize < 1 { return nil, fmt.Errorf("no name given for xattr at position %d", ptr)