Skip to content

Commit 4f05c03

Browse files
weltekialexellis
authored andcommitted
Make symlink extraction in UntarNested conditional
Add an allowSymlinks parameter to UntarNested so callers can opt in to extracting symlinks from a tarball. When disabled, a symlink entry returns an error instead of being created. Wire a --symlinks flag (default false) into the oci install command, passing it through to UntarNested. System installers (go, node, actions_runner, registry, containerd) pass true to preserve existing behaviour for tarballs that contain symlinks. Update existing tests and add a test for the disabled case. Signed-off-by: Han Verstraete (OpenFaaS Ltd) <han@openfaas.com>
1 parent d3faaa6 commit 4f05c03

8 files changed

Lines changed: 50 additions & 21 deletions

File tree

cmd/oci/install.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ OCI image.`,
5656

5757
command.Flags().BoolP("gzipped", "g", false, "Is this a gzipped tarball?")
5858
command.Flags().Bool("quiet", false, "Suppress progress output")
59+
command.Flags().Bool("symlink", false, "Write symlinks when unpacking OCI image, only use with trusted sources")
5960

6061
// Hide the deprecated --path flag
6162
command.Flags().MarkHidden("path")
@@ -68,6 +69,7 @@ OCI image.`,
6869
version, _ := cmd.Flags().GetString("version")
6970
gzipped, _ := cmd.Flags().GetBool("gzipped")
7071
quiet, _ := cmd.Flags().GetBool("quiet")
72+
allowSymlinks, _ := cmd.Flags().GetBool("symlink")
7173
showProgress, _ := cmd.Flags().GetBool("progress")
7274

7375
if len(args) < 1 {
@@ -260,7 +262,7 @@ OCI image.`,
260262
// When the alt-screen is active, suppress UntarNested's
261263
// per-file logging so it doesn't corrupt the live frame.
262264
untarQuiet := quiet || (tty && renderLive)
263-
if uErr := archive.UntarNested(tarFile, installPath, gzipped, untarQuiet); uErr != nil {
265+
if uErr := archive.UntarNested(tarFile, installPath, gzipped, untarQuiet, allowSymlinks); uErr != nil {
264266
workErr = fmt.Errorf("failed to untar %s: %w", tempFile.Name(), uErr)
265267
}
266268
}

cmd/system/actions_runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func MakeInstallActionsRunner() *cobra.Command {
100100
fmt.Printf("Unpacking Actions Runner to: %s\n", path.Join(installPath, "actions-runner"))
101101

102102
if err := spinWhile("Unpacking Actions Runner", func() error {
103-
return archive.UntarNested(f, installPath, true, true)
103+
return archive.UntarNested(f, installPath, true, true, true)
104104
}); err != nil {
105105
return err
106106
}

cmd/system/containerd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func MakeInstallContainerd() *cobra.Command {
120120
tempDirName := os.TempDir() + "/containerd"
121121

122122
if err := spinWhile("Unpacking containerd", func() error {
123-
return archive.UntarNested(f, tempDirName, true, true)
123+
return archive.UntarNested(f, tempDirName, true, true, true)
124124
}); err != nil {
125125
return err
126126
}

cmd/system/go.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func MakeInstallGo() *cobra.Command {
9696
fmt.Printf("Unpacking Go to: %s\n", path.Join(installPath, "go"))
9797

9898
if err := spinWhile("Unpacking Go", func() error {
99-
return archive.UntarNested(f, installPath, true, true)
99+
return archive.UntarNested(f, installPath, true, true, true)
100100
}); err != nil {
101101
return err
102102
}

cmd/system/node.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func MakeInstallNode() *cobra.Command {
149149
fmt.Printf("Unpacking binaries to: %s\n", tempUnpackPath)
150150
}
151151
if err = spinWhile("Unpacking Node.js", func() error {
152-
return archive.UntarNested(f, tempUnpackPath, true, true)
152+
return archive.UntarNested(f, tempUnpackPath, true, true, true)
153153
}); err != nil {
154154
return err
155155
}

cmd/system/registry.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func MakeInstallRegistry() *cobra.Command {
135135
tempDirName := fmt.Sprintf("%s/%s", os.TempDir(), toolName)
136136
defer os.RemoveAll(tempDirName)
137137
if err := spinWhile("Unpacking "+toolName, func() error {
138-
return archive.UntarNested(f, tempDirName, true, true)
138+
return archive.UntarNested(f, tempDirName, true, true, true)
139139
}); err != nil {
140140
return err
141141
}

pkg/archive/untar_nested.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,16 @@ import (
1313
)
1414

1515
// UntarNested reads the gzip-compressed tar file from r and writes it into dir.
16+
// When allowSymlinks is false, any symlink entry in the archive causes an
17+
// error; when true, symlinks are extracted subject to containment checks.
1618
// Copyright 2017 The Go Authors. All rights reserved.
1719
// Use of this source code is governed by a BSD-style
1820
// license that can be found in the LICENSE file.
19-
func UntarNested(r io.Reader, dir string, gzipped, quiet bool) error {
20-
return untarNested(r, dir, gzipped, quiet)
21+
func UntarNested(r io.Reader, dir string, gzipped, quiet, allowSymlinks bool) error {
22+
return untarNested(r, dir, gzipped, quiet, allowSymlinks)
2123
}
2224

23-
func untarNested(r io.Reader, dir string, gzipped, quiet bool) (err error) {
25+
func untarNested(r io.Reader, dir string, gzipped, quiet, allowSymlinks bool) (err error) {
2426
t0 := time.Now()
2527
nFiles := 0
2628
madeDir := map[string]bool{}
@@ -148,6 +150,9 @@ func untarNested(r io.Reader, dir string, gzipped, quiet bool) (err error) {
148150
}
149151
madeDir[abs] = true
150152
case mode.Type() == os.ModeSymlink:
153+
if !allowSymlinks {
154+
return fmt.Errorf("tar file entry %s is a symlink, but symlink extraction is disabled", f.Name)
155+
}
151156
parent := filepath.Dir(abs)
152157
if !madeDir[parent] {
153158
if err := assertExistingPrefixWithinRoot(cleanDir, parent); err != nil {

pkg/archive/untar_nested_test.go

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func Test_UntarNested_RejectsAbsoluteSymlinkWriteThrough(t *testing.T) {
6060
{hdr: tar.Header{Name: "escape-link/escape.txt", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("escaped\n")},
6161
})
6262

63-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
63+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
6464
t.Fatal("want error, got nil")
6565
}
6666

@@ -92,7 +92,7 @@ func Test_UntarNested_RejectsRelativeSymlinkWriteThrough(t *testing.T) {
9292
{hdr: tar.Header{Name: "escape-link/escape.txt", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("escaped\n")},
9393
})
9494

95-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
95+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
9696
t.Fatal("want error, got nil")
9797
}
9898

@@ -121,7 +121,7 @@ func Test_UntarNested_RejectsChainedSymlinkEscape(t *testing.T) {
121121
{hdr: tar.Header{Name: "hop1/hop2/outside/escape.txt", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("escaped\n")},
122122
})
123123

124-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
124+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
125125
t.Fatal("want error, got nil")
126126
}
127127

@@ -149,7 +149,7 @@ func Test_UntarNested_RejectsPlantedEscapingSymlink(t *testing.T) {
149149
{hdr: tar.Header{Name: "hop1/hop2", Typeflag: tar.TypeSymlink, Linkname: "..", Mode: 0777}},
150150
})
151151

152-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
152+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
153153
t.Fatal("want error, got nil")
154154
}
155155

@@ -188,7 +188,7 @@ func Test_UntarNested_RejectsSymlinkTargetViaPreExistingSymlink(t *testing.T) {
188188
{hdr: tar.Header{Name: "planted", Typeflag: tar.TypeSymlink, Linkname: "safe/file", Mode: 0777}},
189189
})
190190

191-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
191+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
192192
t.Fatalf("expected extraction to be rejected, got nil error")
193193
}
194194

@@ -213,7 +213,7 @@ func Test_UntarNested_AllowsValidNestedFiles(t *testing.T) {
213213
{hdr: tar.Header{Name: "README.md", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("hello\n")},
214214
})
215215

216-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err != nil {
216+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err != nil {
217217
t.Fatalf("expected clean extraction, got: %v", err)
218218
}
219219
for _, rel := range []string{"bin/tool", "README.md"} {
@@ -238,7 +238,7 @@ func Test_UntarNested_AllowsWriteThroughInternalSymlink(t *testing.T) {
238238
{hdr: tar.Header{Name: "link/file.txt", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("hello\n")},
239239
})
240240

241-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err != nil {
241+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err != nil {
242242
t.Fatalf("expected clean extraction with write through internal symlink, got: %v", err)
243243
}
244244
if _, err := os.Stat(filepath.Join(installDir, "subdir", "file.txt")); err != nil {
@@ -260,7 +260,7 @@ func Test_UntarNested_AllowsWriteThroughInternalSymlinkDir(t *testing.T) {
260260
{hdr: tar.Header{Name: "link/newdir", Typeflag: tar.TypeDir, Mode: 0755}},
261261
})
262262

263-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err != nil {
263+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err != nil {
264264
t.Fatalf("expected clean extraction with dir write-through internal symlink, got: %v", err)
265265
}
266266
if _, err := os.Stat(filepath.Join(installDir, "subdir", "newdir")); err != nil {
@@ -296,7 +296,7 @@ func Test_UntarNested_PreExistingSymlinkDoesNotCreateDirOutsideRoot(t *testing.T
296296
{hdr: tar.Header{Name: "link/subdir/escape.txt", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("escaped\n")},
297297
})
298298

299-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
299+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
300300
t.Fatal("want error, got nil")
301301
}
302302

@@ -332,7 +332,7 @@ func Test_UntarNested_RejectsLeafSymlinkWriteThrough(t *testing.T) {
332332
{hdr: tar.Header{Name: "evil", Typeflag: tar.TypeReg, Mode: 0644}, body: []byte("HACKED")},
333333
})
334334

335-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err == nil {
335+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err == nil {
336336
t.Fatal("want error, got nil")
337337
}
338338

@@ -354,7 +354,7 @@ func Test_UntarNested_AllowsValidInternalSymlink(t *testing.T) {
354354
{hdr: tar.Header{Name: "tool", Typeflag: tar.TypeSymlink, Linkname: "tool-v1", Mode: 0777}},
355355
})
356356

357-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err != nil {
357+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err != nil {
358358
t.Fatalf("expected clean extraction with internal symlink, got: %v", err)
359359
}
360360
linkPath := filepath.Join(installDir, "tool")
@@ -381,7 +381,7 @@ func Test_UntarNested_CreatesParentDirForSymlink(t *testing.T) {
381381
{hdr: tar.Header{Name: "nested/link", Typeflag: tar.TypeSymlink, Linkname: "tool-v1", Mode: 0777}},
382382
})
383383

384-
if err := UntarNested(bytes.NewReader(data), installDir, false, true); err != nil {
384+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, true); err != nil {
385385
t.Fatalf("expected clean extraction with on-demand parent dir for symlink, got: %v", err)
386386
}
387387
linkPath := filepath.Join(installDir, "nested", "link")
@@ -393,3 +393,25 @@ func Test_UntarNested_CreatesParentDirForSymlink(t *testing.T) {
393393
t.Fatalf("expected %q to be a symlink", linkPath)
394394
}
395395
}
396+
397+
// When allowSymlinks is false, any symlink entry in the tar must be rejected,
398+
// even when the link target stays safely within root.
399+
func Test_UntarNested_RejectsSymlinkWhenDisabled(t *testing.T) {
400+
installDir, err := os.MkdirTemp("", "arkade-untar-*")
401+
if err != nil {
402+
t.Fatal(err)
403+
}
404+
defer os.RemoveAll(installDir)
405+
406+
data := buildTar(t, []tarEntry{
407+
{hdr: tar.Header{Name: "tool-v1", Typeflag: tar.TypeReg, Mode: 0755}, body: []byte("bin\n")},
408+
{hdr: tar.Header{Name: "tool", Typeflag: tar.TypeSymlink, Linkname: "tool-v1", Mode: 0777}},
409+
})
410+
411+
if err := UntarNested(bytes.NewReader(data), installDir, false, true, false); err == nil {
412+
t.Fatalf("expected error when extracting symlink with symlinks disabled, got nil")
413+
}
414+
if _, err := os.Lstat(filepath.Join(installDir, "tool")); err == nil {
415+
t.Fatalf("expected symlink not to be created when symlinks disabled")
416+
}
417+
}

0 commit comments

Comments
 (0)