Skip to content

Commit fd33c05

Browse files
authored
fix: resolve extracted minitiad path and preserve binary permissions (#207)
* fix: resolve extracted minitiad path and preserve binary permissions - Preserve executable permissions during tar extraction so `minitiad` remains runnable after download. - Make binary detection work before executable permissions are restored, then normalize permissions with `chmod`. - Add regression coverage for extracted binary discovery and permission preservation. - Validate the MiniEVM `weave init` flow on both Linux and macOS. * fix: prevent path traversal during archive extraction * fix: harden tar extraction and improve regression tests
1 parent 42070f1 commit fd33c05

File tree

4 files changed

+119
-12
lines changed

4 files changed

+119
-12
lines changed

cosmosutils/binary.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func getMinitiadBinaryURL(vm, version string) (string, error) {
260260
}
261261

262262
// FindBinaryDir walks versionDir to find the directory that contains the named
263-
// executable. This avoids hardcoding assumptions about how a release tarball is
263+
// binary. This avoids hardcoding assumptions about how a release tarball is
264264
// structured, so the code stays correct even if a future tarball places the
265265
// binary inside a subdirectory.
266266
func FindBinaryDir(versionDir, binaryName string) (string, error) {
@@ -269,7 +269,7 @@ func FindBinaryDir(versionDir, binaryName string) (string, error) {
269269
if err != nil {
270270
return err
271271
}
272-
if !info.IsDir() && info.Name() == binaryName && info.Mode()&0o111 != 0 {
272+
if !info.IsDir() && info.Name() == binaryName {
273273
result = filepath.Dir(path)
274274
return filepath.SkipAll
275275
}

cosmosutils/binary_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -341,13 +341,17 @@ func TestFindBinaryDir(t *testing.T) {
341341
wantRel: filepath.Join("a", "b", "c"),
342342
},
343343
{
344-
name: "non-executable file is ignored",
344+
name: "finds binary before executable permissions are restored",
345345
layout: func(root string) {
346-
os.MkdirAll(root, 0o755)
347-
os.WriteFile(filepath.Join(root, "minitiad"), []byte("data"), 0o644)
346+
if err := os.MkdirAll(root, 0o755); err != nil {
347+
t.Fatal(err)
348+
}
349+
if err := os.WriteFile(filepath.Join(root, "minitiad"), []byte("data"), 0o644); err != nil {
350+
t.Fatal(err)
351+
}
348352
},
349353
binaryName: "minitiad",
350-
wantRel: "",
354+
wantRel: ".",
351355
},
352356
{
353357
name: "wrong name is ignored",

io/filesystem.go

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ func DownloadAndExtractTarGz(url, tarballPath, extractedPath string) error {
3838
}
3939

4040
func ExtractTarGz(src string, dest string) error {
41+
destRoot, err := filepath.Abs(dest)
42+
if err != nil {
43+
return err
44+
}
45+
4146
file, err := os.Open(src)
4247
if err != nil {
4348
return err
@@ -60,23 +65,27 @@ func ExtractTarGz(src string, dest string) error {
6065
return err
6166
}
6267

63-
target := filepath.Join(dest, header.Name)
68+
target, err := safeArchivePath(destRoot, header.Name)
69+
if err != nil {
70+
return err
71+
}
6472
switch header.Typeflag {
6573
case tar.TypeDir:
6674
if err := os.MkdirAll(target, os.ModePerm); err != nil {
6775
return err
6876
}
6977
case tar.TypeReg:
70-
file, err := os.Create(target)
71-
if err != nil {
78+
if err := os.MkdirAll(filepath.Dir(target), os.ModePerm); err != nil {
7279
return err
7380
}
74-
_, err = io.Copy(file, tarReader)
81+
file, err := os.OpenFile(target, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, os.FileMode(header.Mode))
7582
if err != nil {
7683
return err
7784
}
78-
err = file.Close()
79-
if err != nil {
85+
if err := writeTarFile(file, tarReader); err != nil {
86+
return err
87+
}
88+
if err := os.Chmod(target, os.FileMode(header.Mode)); err != nil {
8089
return err
8190
}
8291
default:
@@ -86,6 +95,34 @@ func ExtractTarGz(src string, dest string) error {
8695
return nil
8796
}
8897

98+
func writeTarFile(file *os.File, src io.Reader) (err error) {
99+
defer func() {
100+
if closeErr := file.Close(); err == nil && closeErr != nil {
101+
err = closeErr
102+
}
103+
}()
104+
105+
_, err = io.Copy(file, src)
106+
return err
107+
}
108+
109+
func safeArchivePath(destRoot, entryName string) (string, error) {
110+
cleanName := filepath.Clean(entryName)
111+
if cleanName == "." {
112+
return destRoot, nil
113+
}
114+
115+
target := filepath.Join(destRoot, cleanName)
116+
rel, err := filepath.Rel(destRoot, target)
117+
if err != nil {
118+
return "", err
119+
}
120+
if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
121+
return "", fmt.Errorf("unsafe archive entry path: %s", entryName)
122+
}
123+
return target, nil
124+
}
125+
89126
func SetLibraryPaths(binaryDir string) error {
90127
envKey, envValue, err := LibraryPathEnv(binaryDir)
91128
if err != nil {

io/filesystem_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package io
22

33
import (
4+
"archive/tar"
5+
"compress/gzip"
46
"os"
7+
"path/filepath"
58
"testing"
69

710
"github.com/stretchr/testify/assert"
@@ -59,6 +62,69 @@ func TestExtractTarGz(t *testing.T) {
5962
err := ExtractTarGz("./invalid.tar.gz", "./invalid")
6063
assert.Error(t, err)
6164
})
65+
66+
t.Run("PreservesExtractedFileMode", func(t *testing.T) {
67+
tmpDir := t.TempDir()
68+
tarballPath := filepath.Join(tmpDir, "test.tar.gz")
69+
extractDir := filepath.Join(tmpDir, "extract")
70+
71+
file, err := os.Create(tarballPath)
72+
assert.NoError(t, err)
73+
74+
gzw := gzip.NewWriter(file)
75+
tw := tar.NewWriter(gzw)
76+
77+
content := []byte("#!/bin/sh\necho ok\n")
78+
header := &tar.Header{
79+
Name: "minitiad",
80+
Mode: 0o755,
81+
Size: int64(len(content)),
82+
Typeflag: tar.TypeReg,
83+
}
84+
assert.NoError(t, tw.WriteHeader(header))
85+
_, err = tw.Write(content)
86+
assert.NoError(t, err)
87+
assert.NoError(t, tw.Close())
88+
assert.NoError(t, gzw.Close())
89+
assert.NoError(t, file.Close())
90+
91+
err = ExtractTarGz(tarballPath, extractDir)
92+
assert.NoError(t, err)
93+
94+
info, err := os.Stat(filepath.Join(extractDir, "minitiad"))
95+
assert.NoError(t, err)
96+
assert.Equal(t, os.FileMode(0o755), info.Mode().Perm())
97+
})
98+
99+
t.Run("RejectsPathTraversalEntries", func(t *testing.T) {
100+
tmpDir := t.TempDir()
101+
tarballPath := filepath.Join(tmpDir, "test.tar.gz")
102+
extractDir := filepath.Join(tmpDir, "extract")
103+
104+
file, err := os.Create(tarballPath)
105+
assert.NoError(t, err)
106+
107+
gzw := gzip.NewWriter(file)
108+
tw := tar.NewWriter(gzw)
109+
110+
content := []byte("bad\n")
111+
header := &tar.Header{
112+
Name: "../escape",
113+
Mode: 0o644,
114+
Size: int64(len(content)),
115+
Typeflag: tar.TypeReg,
116+
}
117+
assert.NoError(t, tw.WriteHeader(header))
118+
_, err = tw.Write(content)
119+
assert.NoError(t, err)
120+
assert.NoError(t, tw.Close())
121+
assert.NoError(t, gzw.Close())
122+
assert.NoError(t, file.Close())
123+
124+
err = ExtractTarGz(tarballPath, extractDir)
125+
assert.Error(t, err)
126+
assert.Contains(t, err.Error(), "unsafe archive entry path")
127+
})
62128
}
63129

64130
func TestSetLibraryPaths(t *testing.T) {

0 commit comments

Comments
 (0)