Merge pull request #211 from Dvorinka/feat/extract-bomb-guard

Bound cumulative output and entry count on archive extraction

ROADMAP.md

@@ -217,6 +217,7 @@ Order = user-visible value first; each ships with backend + UI + tests.
 - [x] Share `hide_readme` option (upstream #2729 item 6) — `ShareProps.HideReadMe` (only meaningful with `ShowReadMe`); share navigator filters `README.md`/`README.txt` (case-insensitive) from listings while direct-path resolution stays open for the readme viewer; `detectReadMe` URI fallback now probes unconditionally; owner-only `hide_readme` on share responses; Share dialog nested checkbox
 - [x] 2FA recovery codes (upstream #2729 item 3) — `user.two_factor_backup_codes` sensitive JSON of `salt:sha256` digests; `PUT /user/setting/2fa/backup` regenerates 10 one-time codes behind a valid TOTP (rate-limited 5/h); `Verify2FA` falls back to single-use code consumption on TOTP failure; codes invalidated on secret rotation/disable; login phase gains a recovery-code input mode; security settings show remaining count + regenerate dialog
 - [x] Public share directory (upstream #2729 items 4+5) — `share.listed_publicly` opt-in column gated by new `GroupPermissionSharePublicList` group bit; rejected on password-protected shares at the service layer and normalized off at creation; `GET /share/listed` anonymous endpoint (rate-limited 60/min/IP, cursor pagination) listing only non-expired passwordless listed shares with case-insensitive name search across anchor + covered files; `listed_publicly` owner-visible in share responses; `/discover` page (anonymous-visible nav item + sign-in link), admin group Share section switch, en+zh locales
+- [x] Decompression-bomb guards (meta #2 item 10) — `DecompressSize` now bounds cumulative extracted output (its documented "total file size" intent), not just compressed input: `checkExtractGuards` aborts at the limit and at a 100k-entry cap (`maxExtractEntries`, bounds dir-creation bombs), each entry stream wrapped in `cappedFile` so understated size headers cannot overrun; slave path receives the limit via `SlaveExtractArchiveTaskState.ExtractLimit`; all failures carry `queue.CriticalErr` (no retry of the same bomb); resume-safe via cursor-skip size accounting
 
 ## 6. Phase D — desktop, all platforms
 

pkg/filemanager/workflows/extract.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	iofs "io/fs"
 	"os"
 	"path"
 	"path/filepath"
@@ -65,6 +66,12 @@ const (
 	ProgressTypeExtractSize  = "extract_size"
 	ProgressTypeDownload     = "download"
 
+	// maxExtractEntries bounds the number of archive entries a single
+	// extraction task will process. Archives with more entries than this
+	// abort — a decompression bomb with millions of tiny entries would
+	// otherwise flood the file table.
+	maxExtractEntries int64 = 100_000
+
 	SummaryKeySrc         = "src"
 	SummaryKeySrcPhysical = "src_physical"
 	SummaryKeyDst         = "dst"
@@ -212,15 +219,16 @@ func (m *ExtractArchiveTask) createSlaveExtractTask(ctx context.Context, dep dep
 	}
 
 	payload := &SlaveExtractArchiveTaskState{
-		FileName: archiveFile.DisplayName(),
-		Entity:   entityModel,
-		Policy:   policy,
-		Encoding: m.state.Encoding,
-		Dst:      m.state.Dst,
-		UserID:   user.ID,
-		Password: m.state.Password,
-		FileMask: m.state.FileMask,
-		Volumes:  m.resolveVolumeEntities(ctx, fm, uri.DirUri(), archiveFile.DisplayName(), entityModel),
+		FileName:     archiveFile.DisplayName(),
+		Entity:       entityModel,
+		Policy:       policy,
+		Encoding:     m.state.Encoding,
+		Dst:          m.state.Dst,
+		UserID:       user.ID,
+		ExtractLimit: user.Edges.Group.Settings.DecompressSize,
+		Password:     m.state.Password,
+		FileMask:     m.state.FileMask,
+		Volumes:      m.resolveVolumeEntities(ctx, fm, uri.DirUri(), archiveFile.DisplayName(), entityModel),
 	}
 
 	payloadStr, err := json.Marshal(payload)
@@ -424,6 +432,13 @@ func (m *ExtractArchiveTask) masterExtractArchive(ctx context.Context, dep depen
 			return nil
 		}
 
+		// Decompression-bomb guard: abort when cumulative output or entry
+		// count exceeds the group's bounds.
+		sizeLimit := user.Edges.Group.Settings.DecompressSize
+		if err := checkExtractGuards(m.progress, sizeLimit); err != nil {
+			return err
+		}
+
 		if f.FileInfo.IsDir() {
 			_, err := fm.Create(ctx, savePath, types.FileTypeFolder)
 			if err != nil {
@@ -441,6 +456,13 @@ func (m *ExtractArchiveTask) masterExtractArchive(ctx context.Context, dep depen
 			return nil
 		}
 
+		if sizeLimit > 0 {
+			// Declared entry sizes are advisory; cap the stream at the
+			// remaining budget so understated sizes cannot overrun.
+			remaining := sizeLimit - atomic.LoadInt64(&m.progress[ProgressTypeExtractSize].Current)
+			fileStream = &cappedFile{File: fileStream, remaining: remaining}
+		}
+
 		fileData := &fs.UploadRequest{
 			Props: &fs.UploadProps{
 				Uri:  savePath,
@@ -660,6 +682,7 @@ type (
 		Encoding        string                 `json:"encoding,omitempty"`
 		Dst             string                 `json:"dst,omitempty"`
 		UserID          int                    `json:"user_id"`
+		ExtractLimit    int64                  `json:"extract_limit,omitempty"`
 		TempPath        string                 `json:"temp_path,omitempty"`
 		TempZipFilePath string                 `json:"temp_zip_file_path,omitempty"`
 		ProcessedCursor string                 `json:"processed_cursor,omitempty"`
@@ -870,6 +893,10 @@ func (m *SlaveExtractArchiveTask) Do(ctx context.Context) (task.Status, error) {
 			return nil
 		}
 
+		if err := checkExtractGuards(m.progress, m.state.ExtractLimit); err != nil {
+			return err
+		}
+
 		if f.FileInfo.IsDir() {
 			_, err := fm.Create(ctx, savePath, types.FileTypeFolder, fs.WithNode(m.node), fs.WithStatelessUserID(m.state.UserID))
 			if err != nil {
@@ -887,6 +914,11 @@ func (m *SlaveExtractArchiveTask) Do(ctx context.Context) (task.Status, error) {
 			return nil
 		}
 
+		if m.state.ExtractLimit > 0 {
+			remaining := m.state.ExtractLimit - atomic.LoadInt64(&m.progress[ProgressTypeExtractSize].Current)
+			fileStream = &cappedFile{File: fileStream, remaining: remaining}
+		}
+
 		fileData := &fs.UploadRequest{
 			Props: &fs.UploadProps{
 				Uri:  savePath,
@@ -947,3 +979,55 @@ func isFileInMask(path string, mask []string) bool {
 
 	return false
 }
+
+// errExtractSizeLimit aborts extraction when cumulative decompressed output
+// exceeds the group's DecompressSize bound. It carries CriticalErr so retries
+// do not reprocess the same bomb.
+var errExtractSizeLimit = fmt.Errorf("extracted size exceeds the decompress limit: %w", queue.CriticalErr)
+
+// checkExtractGuards enforces the decompression-bomb bounds before an archive
+// entry is written: cumulative output size (declared, from the shared progress
+// counter) and total entry count. A returned error aborts the task as a
+// critical failure — no retry will change the outcome.
+func checkExtractGuards(progress queue.Progresses, sizeLimit int64) error {
+	if sizeLimit > 0 {
+		current := atomic.LoadInt64(&progress[ProgressTypeExtractSize].Current)
+		if current >= sizeLimit {
+			return fmt.Errorf("%w (%d >= %d)", errExtractSizeLimit, current, sizeLimit)
+		}
+	}
+	if count := atomic.LoadInt64(&progress[ProgressTypeExtractCount].Current); count >= maxExtractEntries {
+		return fmt.Errorf("archive exceeds the entry limit %d: %w", maxExtractEntries, queue.CriticalErr)
+	}
+	return nil
+}
+
+// cappedFile bounds a single archive entry's stream at `remaining`
+// bytes. Declared entry sizes are advisory — a crafted archive can understate
+// them — so the stream itself is capped; reading past the budget fails the
+// upload and aborts extraction.
+type cappedFile struct {
+	iofs.File
+	remaining int64
+}
+
+func (c *cappedFile) Read(p []byte) (int, error) {
+	if len(p) == 0 {
+		return 0, nil
+	}
+	if c.remaining <= 0 {
+		// Budget exhausted: an entry ending exactly at the boundary must
+		// still see EOF, while any further data fails the upload.
+		n, err := c.File.Read(p[:1])
+		if n > 0 {
+			return 0, errExtractSizeLimit
+		}
+		return 0, err
+	}
+	if int64(len(p)) > c.remaining {
+		p = p[:c.remaining]
+	}
+	n, err := c.File.Read(p)
+	c.remaining -= int64(n)
+	return n, err
+}

pkg/filemanager/workflows/extract_test.go

@@ -0,0 +1,92 @@
+package workflows
+
+import (
+	"errors"
+	"io"
+	iofs "io/fs"
+	"strings"
+	"testing"
+
+	"github.com/cloudreve/Cloudreve/v4/pkg/queue"
+	"github.com/stretchr/testify/require"
+)
+
+func testProgress(count, size int64) queue.Progresses {
+	return queue.Progresses{
+		ProgressTypeExtractCount: &queue.Progress{Current: count},
+		ProgressTypeExtractSize:  &queue.Progress{Current: size},
+	}
+}
+
+func TestCheckExtractGuards(t *testing.T) {
+	// Under all limits.
+	require.NoError(t, checkExtractGuards(testProgress(10, 100), 1000))
+
+	// At the cumulative size limit — aborts as non-retryable.
+	err := checkExtractGuards(testProgress(10, 1000), 1000)
+	require.Error(t, err)
+	require.True(t, errors.Is(err, queue.CriticalErr))
+
+	// At the entry cap — aborts as non-retryable.
+	err = checkExtractGuards(testProgress(maxExtractEntries, 10), 1000)
+	require.Error(t, err)
+	require.True(t, errors.Is(err, queue.CriticalErr))
+
+	// Zero size limit disables the size bound; entry cap still applies.
+	require.NoError(t, checkExtractGuards(testProgress(10, 1<<62), 0))
+	require.Error(t, checkExtractGuards(testProgress(maxExtractEntries, 0), 0))
+}
+
+type stubFile struct {
+	io.Reader
+}
+
+func (stubFile) Stat() (iofs.FileInfo, error) { return nil, nil }
+func (stubFile) Close() error                 { return nil }
+
+func TestCappedFileExactBoundary(t *testing.T) {
+	// Entry ends exactly at the budget — stream must terminate with EOF.
+	capped := &cappedFile{File: stubFile{strings.NewReader("12345")}, remaining: 5}
+	buf := make([]byte, 8)
+
+	n, err := capped.Read(buf)
+	require.NoError(t, err)
+	require.Equal(t, 5, n)
+	require.Equal(t, "12345", string(buf[:n]))
+
+	_, err = capped.Read(buf)
+	require.Equal(t, io.EOF, err)
+}
+
+func TestCappedFileBomb(t *testing.T) {
+	// Entry data beyond the budget — read fails with the critical limit error.
+	capped := &cappedFile{File: stubFile{strings.NewReader("123456")}, remaining: 5}
+	buf := make([]byte, 8)
+
+	n, err := capped.Read(buf)
+	require.NoError(t, err)
+	require.Equal(t, 5, n)
+
+	_, err = capped.Read(buf)
+	require.Error(t, err)
+	require.True(t, errors.Is(err, queue.CriticalErr))
+}
+
+func TestCappedFileShortReads(t *testing.T) {
+	// Budget caps each read; consecutive reads drain only the remaining bytes.
+	capped := &cappedFile{File: stubFile{strings.NewReader("abcdef")}, remaining: 3}
+	buf := make([]byte, 2)
+
+	var got []byte
+	var err error
+	for {
+		var n int
+		n, err = capped.Read(buf)
+		got = append(got, buf[:n]...)
+		if err != nil {
+			break
+		}
+	}
+	require.Equal(t, "abc", string(got))
+	require.True(t, errors.Is(err, queue.CriticalErr))
+}