Harden Windows package paths (T-605)
This commit is contained in:
+72
-47
@@ -7,10 +7,10 @@ import (
|
||||
"io"
|
||||
"math"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"softbox.local/core/internal/safepath"
|
||||
)
|
||||
|
||||
var (
|
||||
@@ -45,6 +45,7 @@ type plannedEntry struct {
|
||||
file *zip.File
|
||||
archivePath string
|
||||
outputPath string
|
||||
targetPath string
|
||||
directory bool
|
||||
}
|
||||
|
||||
@@ -85,11 +86,19 @@ func (extractor Extractor) extract(
|
||||
if err != nil {
|
||||
return ExtractResult{}, err
|
||||
}
|
||||
destinationRoot, entrypointPath, err := planOutputPaths(
|
||||
destination,
|
||||
normalizedEntrypoint,
|
||||
plan,
|
||||
)
|
||||
if err != nil {
|
||||
return ExtractResult{}, err
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(filepath.Dir(destination), 0o700); err != nil {
|
||||
if err := os.MkdirAll(filepath.Dir(destinationRoot), 0o700); err != nil {
|
||||
return ExtractResult{}, fmt.Errorf("create staging parent: %w", err)
|
||||
}
|
||||
if err := os.Mkdir(destination, 0o700); err != nil {
|
||||
if err := os.Mkdir(destinationRoot, 0o700); err != nil {
|
||||
if os.IsExist(err) {
|
||||
return ExtractResult{}, ErrDestinationExists
|
||||
}
|
||||
@@ -98,23 +107,22 @@ func (extractor Extractor) extract(
|
||||
complete := false
|
||||
defer func() {
|
||||
if !complete {
|
||||
_ = os.RemoveAll(destination)
|
||||
_ = os.RemoveAll(destinationRoot)
|
||||
}
|
||||
}()
|
||||
|
||||
var written int64
|
||||
for _, entry := range plan {
|
||||
target := filepath.Join(destination, filepath.FromSlash(entry.outputPath))
|
||||
if entry.directory {
|
||||
if entry.outputPath == "" {
|
||||
continue
|
||||
}
|
||||
if err := os.MkdirAll(target, 0o700); err != nil {
|
||||
if err := os.MkdirAll(entry.targetPath, 0o700); err != nil {
|
||||
return ExtractResult{}, fmt.Errorf("create staging directory: %w", err)
|
||||
}
|
||||
continue
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Dir(target), 0o700); err != nil {
|
||||
if err := os.MkdirAll(filepath.Dir(entry.targetPath), 0o700); err != nil {
|
||||
return ExtractResult{}, fmt.Errorf("create staging file parent: %w", err)
|
||||
}
|
||||
|
||||
@@ -126,7 +134,11 @@ func (extractor Extractor) extract(
|
||||
if entry.file.Mode().Perm()&0o111 != 0 {
|
||||
mode = 0o700
|
||||
}
|
||||
output, err := os.OpenFile(target, os.O_CREATE|os.O_EXCL|os.O_WRONLY, mode)
|
||||
output, err := os.OpenFile(
|
||||
entry.targetPath,
|
||||
os.O_CREATE|os.O_EXCL|os.O_WRONLY,
|
||||
mode,
|
||||
)
|
||||
if err != nil {
|
||||
source.Close()
|
||||
return ExtractResult{}, fmt.Errorf("create staging file: %w", err)
|
||||
@@ -166,14 +178,52 @@ func (extractor Extractor) extract(
|
||||
}
|
||||
|
||||
result.Bytes = written
|
||||
result.EntrypointPath = filepath.Join(
|
||||
destination,
|
||||
filepath.FromSlash(normalizedEntrypoint),
|
||||
)
|
||||
result.EntrypointPath = entrypointPath
|
||||
complete = true
|
||||
return result, nil
|
||||
}
|
||||
|
||||
func planOutputPaths(
|
||||
destination string,
|
||||
entrypoint string,
|
||||
plan []plannedEntry,
|
||||
) (string, string, error) {
|
||||
if destination == "" {
|
||||
return "", "", fmt.Errorf("%w: staging destination is empty", ErrPathEscape)
|
||||
}
|
||||
destinationRoot, err := filepath.Abs(destination)
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("%w: resolve staging destination: %v", ErrPathEscape, err)
|
||||
}
|
||||
destinationRoot = filepath.Clean(destinationRoot)
|
||||
for index := range plan {
|
||||
if plan[index].outputPath == "" {
|
||||
plan[index].targetPath = destinationRoot
|
||||
continue
|
||||
}
|
||||
target, err := safepath.JoinUnder(destinationRoot, plan[index].outputPath)
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf(
|
||||
"%w: output %q: %v",
|
||||
ErrPathEscape,
|
||||
plan[index].outputPath,
|
||||
err,
|
||||
)
|
||||
}
|
||||
plan[index].targetPath = target
|
||||
}
|
||||
entrypointPath, err := safepath.JoinUnder(destinationRoot, entrypoint)
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf(
|
||||
"%w: entrypoint %q: %v",
|
||||
ErrEntrypointInvalid,
|
||||
entrypoint,
|
||||
err,
|
||||
)
|
||||
}
|
||||
return destinationRoot, entrypointPath, nil
|
||||
}
|
||||
|
||||
func (extractor Extractor) preflight(
|
||||
archive *zip.Reader,
|
||||
entrypoint string,
|
||||
@@ -200,7 +250,7 @@ func (extractor Extractor) preflight(
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
folded := strings.ToLower(normalized)
|
||||
folded := safepath.CollisionKey(normalized)
|
||||
if previous, exists := seenPaths[folded]; exists {
|
||||
return nil, fmt.Errorf(
|
||||
"%w: %q conflicts with %q",
|
||||
@@ -305,47 +355,22 @@ func validateArchiveEntry(file *zip.File) (string, bool, error) {
|
||||
}
|
||||
|
||||
func normalizeArchivePath(name string) (string, bool, error) {
|
||||
if name == "" || !utf8.ValidString(name) || strings.ContainsRune(name, '\x00') {
|
||||
return "", false, fmt.Errorf("%w: invalid entry name", ErrPathEscape)
|
||||
}
|
||||
if strings.Contains(name, `\`) || strings.Contains(name, ":") {
|
||||
return "", false, fmt.Errorf("%w: %q", ErrPathEscape, name)
|
||||
}
|
||||
directory := strings.HasSuffix(name, "/")
|
||||
trimmed := strings.TrimSuffix(name, "/")
|
||||
if trimmed == "" || path.IsAbs(trimmed) || strings.HasPrefix(trimmed, "/") {
|
||||
return "", false, fmt.Errorf("%w: %q", ErrPathEscape, name)
|
||||
if err := safepath.ValidateRelative(trimmed); err != nil {
|
||||
return "", false, fmt.Errorf("%w: %q: %v", ErrPathEscape, name, err)
|
||||
}
|
||||
cleaned := path.Clean(trimmed)
|
||||
if cleaned != trimmed ||
|
||||
cleaned == "." ||
|
||||
cleaned == ".." ||
|
||||
strings.HasPrefix(cleaned, "../") {
|
||||
return "", false, fmt.Errorf("%w: %q", ErrPathEscape, name)
|
||||
}
|
||||
return cleaned, directory, nil
|
||||
return trimmed, directory, nil
|
||||
}
|
||||
|
||||
func normalizeEntrypoint(entrypoint string) (string, error) {
|
||||
if entrypoint == "" ||
|
||||
!utf8.ValidString(entrypoint) ||
|
||||
strings.ContainsRune(entrypoint, '\x00') ||
|
||||
strings.Contains(entrypoint, `\`) ||
|
||||
strings.Contains(entrypoint, ":") ||
|
||||
strings.HasSuffix(entrypoint, "/") ||
|
||||
path.IsAbs(entrypoint) {
|
||||
if err := safepath.ValidateRelative(entrypoint); err != nil {
|
||||
return "", fmt.Errorf("%w: %q: %v", ErrEntrypointInvalid, entrypoint, err)
|
||||
}
|
||||
if entrypoint == "payload" || strings.HasPrefix(entrypoint, "payload/") {
|
||||
return "", fmt.Errorf("%w: %q", ErrEntrypointInvalid, entrypoint)
|
||||
}
|
||||
cleaned := path.Clean(entrypoint)
|
||||
if cleaned != entrypoint ||
|
||||
cleaned == "." ||
|
||||
cleaned == ".." ||
|
||||
strings.HasPrefix(cleaned, "../") ||
|
||||
cleaned == "payload" ||
|
||||
strings.HasPrefix(cleaned, "payload/") {
|
||||
return "", fmt.Errorf("%w: %q", ErrEntrypointInvalid, entrypoint)
|
||||
}
|
||||
return cleaned, nil
|
||||
return entrypoint, nil
|
||||
}
|
||||
|
||||
func exceedsCompressionRatio(uncompressed, compressed uint64, maximum float64) bool {
|
||||
|
||||
@@ -83,6 +83,70 @@ func TestExtractorRejectsAttackArchives(t *testing.T) {
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "Windows normalized dot dot traversal",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/.. /escape.exe", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "trailing period",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/evil.exe.", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "trailing space",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/evil.exe ", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "leading space",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/ evil.exe", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "reserved device",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/NUL.txt", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "reserved device with space before extension",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/NUL .txt", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "reserved device superscript",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/COM¹.log", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "forbidden Windows character",
|
||||
entries: appendEntries(base,
|
||||
testZIPEntry{name: "payload/evil?.exe", body: []byte("x")}),
|
||||
entrypoint: "App.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrPathEscape,
|
||||
},
|
||||
{
|
||||
name: "backslash traversal",
|
||||
entries: appendEntries(base,
|
||||
@@ -127,6 +191,17 @@ func TestExtractorRejectsAttackArchives(t *testing.T) {
|
||||
limits: testLimits(),
|
||||
wantErr: ErrDuplicateEntry,
|
||||
},
|
||||
{
|
||||
name: "Unicode folded duplicate",
|
||||
entries: []testZIPEntry{
|
||||
{name: "app.json", body: []byte(`{}`)},
|
||||
{name: "payload/K.exe", body: []byte("one")},
|
||||
{name: "payload/K.exe", body: []byte("two")},
|
||||
},
|
||||
entrypoint: "K.exe",
|
||||
limits: testLimits(),
|
||||
wantErr: ErrDuplicateEntry,
|
||||
},
|
||||
{
|
||||
name: "unexpected top level entry",
|
||||
entries: appendEntries(base,
|
||||
@@ -184,7 +259,8 @@ func TestExtractorRejectsAttackArchives(t *testing.T) {
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
archivePath := writeTestZIP(t, test.entries)
|
||||
destination := filepath.Join(t.TempDir(), "staging")
|
||||
root := t.TempDir()
|
||||
destination := filepath.Join(root, "staging")
|
||||
extractor := mustExtractor(t, test.limits)
|
||||
|
||||
_, err := extractor.ExtractFile(archivePath, destination, test.entrypoint)
|
||||
@@ -194,6 +270,9 @@ func TestExtractorRejectsAttackArchives(t *testing.T) {
|
||||
if _, statErr := os.Stat(destination); !os.IsNotExist(statErr) {
|
||||
t.Fatalf("rejected archive left staging, stat error = %v", statErr)
|
||||
}
|
||||
if _, statErr := os.Stat(filepath.Join(root, "escape.exe")); !os.IsNotExist(statErr) {
|
||||
t.Fatalf("rejected archive wrote outside staging, stat error = %v", statErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -210,6 +289,12 @@ func TestExtractorRejectsInvalidEntrypoints(t *testing.T) {
|
||||
{entrypoint: "../App.exe", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "/App.exe", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: `..\App.exe`, wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: ".. /App.exe", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "App.exe.", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "App.exe ", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: " App.exe", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "NUL", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "bad?.exe", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "payload/App.exe", wantErr: ErrEntrypointInvalid},
|
||||
{entrypoint: "Missing.exe", wantErr: ErrEntrypointMissing},
|
||||
}
|
||||
@@ -229,6 +314,27 @@ func TestExtractorRejectsInvalidEntrypoints(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractorAcceptsUnicodeNestedPaths(t *testing.T) {
|
||||
archivePath := writeTestZIP(t, []testZIPEntry{
|
||||
{name: "app.json", body: []byte(`{}`)},
|
||||
{name: "payload/工具/解析器.exe", body: []byte("executable")},
|
||||
{name: "payload/资源/说明.txt", body: []byte("说明")},
|
||||
})
|
||||
destination := filepath.Join(t.TempDir(), "staging")
|
||||
extractor := mustExtractor(t, testLimits())
|
||||
|
||||
result, err := extractor.ExtractFile(archivePath, destination, "工具/解析器.exe")
|
||||
if err != nil {
|
||||
t.Fatalf("ExtractFile() error = %v", err)
|
||||
}
|
||||
if result.Files != 2 {
|
||||
t.Fatalf("Files = %d, want 2", result.Files)
|
||||
}
|
||||
if _, err := os.Stat(result.EntrypointPath); err != nil {
|
||||
t.Fatalf("entrypoint stat error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractorRejectsExistingDestination(t *testing.T) {
|
||||
archivePath := writeTestZIP(t, []testZIPEntry{
|
||||
{name: "app.json", body: []byte(`{}`)},
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
//go:build windows
|
||||
|
||||
package installer
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestExtractorRejectsWindowsNormalizedEscapeOnNativeFilesystem(t *testing.T) {
|
||||
archivePath := writeTestZIP(t, []testZIPEntry{
|
||||
{name: "app.json", body: []byte(`{}`)},
|
||||
{name: "payload/App.exe", body: []byte("ok")},
|
||||
{name: "payload/.. /escape.exe", body: []byte("escape")},
|
||||
})
|
||||
root := t.TempDir()
|
||||
destination := filepath.Join(root, "staging")
|
||||
extractor := mustExtractor(t, testLimits())
|
||||
|
||||
_, err := extractor.ExtractFile(archivePath, destination, "App.exe")
|
||||
if !errors.Is(err, ErrPathEscape) {
|
||||
t.Fatalf("ExtractFile() error = %v, want %v", err, ErrPathEscape)
|
||||
}
|
||||
if _, statErr := os.Stat(destination); !os.IsNotExist(statErr) {
|
||||
t.Fatalf("rejected archive left staging, stat error = %v", statErr)
|
||||
}
|
||||
if _, statErr := os.Stat(filepath.Join(root, "escape.exe")); !os.IsNotExist(statErr) {
|
||||
t.Fatalf("archive escaped staging, stat error = %v", statErr)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user