Split .tar.gz extraction into installer and TUF implementations to remove permissions checks on installer implementation (#28888)
For #26692 (fixes permission issue when extracting dirs). Reverts changes to `update.go` to remove TUF test surface. # Checklist for submitter If some of the following don't apply, delete the relevant line. <!-- Note that API documentation changes are now addressed by the product design team. --> - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it) - [x] Manual QA for all new/changed functionality - For Orbit and Fleet Desktop changes: - [ ] Make sure fleetd is compatible with the latest released version of Fleet (see [Must rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/fleetd-development-and-release-strategy.md)). - [ ] Orbit runs on macOS, Linux and Windows. Check if the orbit feature/bugfix should only apply to one platform (`runtime.GOOS`). - [ ] Manual QA must be performed in the three main OSs, macOS, Windows and Linux. - [ ] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)).
This commit is contained in:
@@ -334,17 +334,7 @@ func (r *Runner) installSoftware(ctx context.Context, installID string, logger z
|
||||
extractFn := r.extractTarGzFn
|
||||
if extractFn == nil {
|
||||
extractFn = func(path string, destDir string) error {
|
||||
tarGzFile, err := os.Open(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("oepn file for extraction: %w", err)
|
||||
}
|
||||
defer tarGzFile.Close()
|
||||
|
||||
if err = update.ExtractOpenTarGzFile(tarGzFile, destDir); err != nil {
|
||||
return fmt.Errorf("extract %q: %w", path, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
return file.ExtractTarGz(path, destDir, 2*1024*1024*1024*1024) // 2 TiB limit per extracted file
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -681,7 +681,7 @@ func (u *Updater) checkExec(target, tmpPath string, customCheckExec func(execPat
|
||||
return nil
|
||||
}
|
||||
|
||||
// extractTarGz extracts the contents of the provided tar.gz file.
|
||||
// extractTagGz extracts the contents of the provided tar.gz file.
|
||||
func extractTarGz(path string) error {
|
||||
tarGzFile, err := secure.OpenFile(path, os.O_RDONLY, 0o755)
|
||||
if err != nil {
|
||||
@@ -689,17 +689,9 @@ func extractTarGz(path string) error {
|
||||
}
|
||||
defer tarGzFile.Close()
|
||||
|
||||
if err = ExtractOpenTarGzFile(tarGzFile, filepath.Dir(path)); err != nil {
|
||||
return fmt.Errorf("extract %q: %w", path, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func ExtractOpenTarGzFile(tarGzFile *os.File, destDir string) error {
|
||||
gzipReader, err := gzip.NewReader(tarGzFile)
|
||||
if err != nil {
|
||||
return fmt.Errorf("gzip reader: %w", err)
|
||||
return fmt.Errorf("gzip reader %q: %w", path, err)
|
||||
}
|
||||
defer gzipReader.Close()
|
||||
|
||||
@@ -712,7 +704,7 @@ func ExtractOpenTarGzFile(tarGzFile *os.File, destDir string) error {
|
||||
case errors.Is(err, io.EOF):
|
||||
return nil
|
||||
default:
|
||||
return fmt.Errorf("tar reader: %w", err)
|
||||
return fmt.Errorf("tar reader %q: %w", path, err)
|
||||
}
|
||||
|
||||
// Prevent zip-slip attack.
|
||||
@@ -720,7 +712,7 @@ func ExtractOpenTarGzFile(tarGzFile *os.File, destDir string) error {
|
||||
return fmt.Errorf("invalid path in tar.gz: %q", header.Name)
|
||||
}
|
||||
|
||||
targetPath := filepath.Join(destDir, header.Name)
|
||||
targetPath := filepath.Join(filepath.Dir(path), header.Name)
|
||||
|
||||
switch header.Typeflag {
|
||||
case tar.TypeDir:
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
package file
|
||||
|
||||
import (
|
||||
"archive/tar"
|
||||
"bufio"
|
||||
"bytes"
|
||||
"compress/gzip"
|
||||
"encoding/binary"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -14,6 +16,7 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/orbit/pkg/constant"
|
||||
"github.com/fleetdm/fleet/v4/pkg/secure"
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
)
|
||||
@@ -193,3 +196,87 @@ func ExtractFilenameFromURLPath(p string, defaultExtension string) string {
|
||||
|
||||
return b
|
||||
}
|
||||
|
||||
// ExtractTarGz extracts the contents of the provided tar.gz file.
|
||||
// This implementation uses os.* calls without permission checks, as we're
|
||||
// running this operation in the context of fleetd running as root on a host
|
||||
// (e.g. for installs), so we have different constraints than fleetctl building
|
||||
// a package. destDir should be provided by the code rather than user input to
|
||||
// avoid directory traversal attacks. maxFileSize indicates how large we want
|
||||
// to allow the max file size to be when decompressing, as a zip bomb mitigation.
|
||||
func ExtractTarGz(path string, destDir string, maxFileSize int64) error {
|
||||
tarGzFile, err := os.Open(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open %q: %w", path, err)
|
||||
}
|
||||
defer tarGzFile.Close()
|
||||
|
||||
gzipReader, err := gzip.NewReader(tarGzFile)
|
||||
if err != nil {
|
||||
return fmt.Errorf("gzip reader: %w", err)
|
||||
}
|
||||
defer gzipReader.Close()
|
||||
|
||||
tarReader := tar.NewReader(gzipReader)
|
||||
|
||||
for {
|
||||
header, err := tarReader.Next()
|
||||
switch {
|
||||
case err == nil:
|
||||
// OK
|
||||
case errors.Is(err, io.EOF):
|
||||
return nil
|
||||
default:
|
||||
return fmt.Errorf("tar reader: %w", err)
|
||||
}
|
||||
|
||||
// Prevent zip-slip attack (which, combined with a trusted destDir, remediates the potential directory traversal
|
||||
// attack below)
|
||||
if strings.Contains(header.Name, "..") {
|
||||
return fmt.Errorf("invalid path in tar.gz: %q", header.Name)
|
||||
}
|
||||
|
||||
targetPath := filepath.Join(destDir, header.Name) // nolint:gosec // see above notes on dir traversal
|
||||
|
||||
switch header.Typeflag {
|
||||
case tar.TypeDir:
|
||||
if err := os.MkdirAll(targetPath, constant.DefaultDirMode); err != nil {
|
||||
return fmt.Errorf("mkdir %q: %w", header.Name, err)
|
||||
}
|
||||
case tar.TypeReg:
|
||||
err := func() error {
|
||||
outFile, err := os.OpenFile(targetPath, os.O_CREATE|os.O_WRONLY, header.FileInfo().Mode())
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create %q: %w", header.Name, err)
|
||||
}
|
||||
defer outFile.Close()
|
||||
|
||||
// CopyN call to avoid zip bomb DoS since we have less control over arbitrary .tar.gz archives
|
||||
// than in e.g. a TUF case.
|
||||
var readBytes int64
|
||||
chunkSize := int64(65536) // 64KiB
|
||||
for {
|
||||
if readBytes+chunkSize > maxFileSize {
|
||||
return fmt.Errorf("aborted extraction of oversized file after %d bytes", readBytes)
|
||||
}
|
||||
|
||||
_, err := io.CopyN(outFile, tarReader, chunkSize)
|
||||
if err != nil {
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
return fmt.Errorf("failed to extract file %q inside %q: %w", header.Name, path, err)
|
||||
}
|
||||
readBytes += chunkSize
|
||||
}
|
||||
|
||||
return nil
|
||||
}()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
default:
|
||||
return fmt.Errorf("unknown flag type %q: %d", header.Name, header.Typeflag)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user