Skip to content

Commit

Permalink
fix: a bunch of bugs with scaffolding (#514)
Browse files Browse the repository at this point in the history
- Fixes "tainted" zip paths
- Fixes `go mod tidy` failing against the scaffolding go.mod.tmpl
  • Loading branch information
alecthomas authored Oct 20, 2023
1 parent 5bdf72c commit eef7f10
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 33 deletions.
8 changes: 1 addition & 7 deletions cmd/ftl/cmd_init.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package main

import (
"context"
"os"
"path/filepath"

"github.com/alecthomas/errors"

"github.com/TBD54566975/ftl/backend/common/exec"
"github.com/TBD54566975/ftl/backend/common/log"
goruntime "github.com/TBD54566975/ftl/go-runtime"
"github.com/TBD54566975/ftl/internal"
kotlinruntime "github.com/TBD54566975/ftl/kotlin-runtime"
Expand All @@ -26,7 +23,7 @@ type initGoCmd struct {
GoModule string `short:"G" required:"" help:"Go module import path."`
}

func (i initGoCmd) Run(ctx context.Context, parent *initCmd) error {
func (i initGoCmd) Run(parent *initCmd) error {
if i.Name == "" {
i.Name = filepath.Base(i.Dir)
}
Expand All @@ -42,9 +39,6 @@ func (i initGoCmd) Run(ctx context.Context, parent *initCmd) error {
return errors.WithStack(err)
}
}
if err := exec.Command(ctx, log.Info, i.Dir, "go", "mod", "tidy").Run(); err != nil {
return errors.WithStack(err)
}
return nil
}

Expand Down
File renamed without changes.
13 changes: 12 additions & 1 deletion internal/scaffolder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
//
// Both paths and file contents are evaluated.
//
// If a file name ends with ".tmpl", the ".tmpl" suffix is removed.
//
// The functions "snake", "camel", "lowerCamel", "kebab", "upper", and "lower"
// are available.
func Scaffold(destination string, ctx any) error {
Expand All @@ -26,7 +28,15 @@ func Scaffold(destination string, ctx any) error {
return errors.WithStack(err)
}

// Evaluate path name templates.
if strings.HasSuffix(path, ".tmpl") {
newPath := strings.TrimSuffix(path, ".tmpl")
if err = os.Rename(path, newPath); err != nil {
return errors.Wrap(err, "failed to rename file")
}
path = newPath
}

// Evaluate the last component of path name templates.
dir := filepath.Dir(path)
base := filepath.Base(path)
newName, err := evaluate(base, ctx)
Expand Down Expand Up @@ -64,6 +74,7 @@ func Scaffold(destination string, ctx any) error {
}))
}

// Walk dir executing fn after each entry.
func walkDir(dir string, fn func(path string, d fs.DirEntry) error) error {
entries, err := os.ReadDir(dir)
if err != nil {
Expand Down
29 changes: 4 additions & 25 deletions internal/zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package internal
import (
"archive/zip"
"bytes"
"fmt"
"io"
"os"
"path/filepath"
Expand All @@ -19,11 +18,10 @@ func UnzipDir(zipReader *zip.Reader, destDir string) error {
return errors.WithStack(err)
}
for _, file := range zipReader.File {
destPath, err := sanitizeArchivePath(destDir, file.Name)
if err != nil {
return errors.WithStack(err)
destPath := filepath.Clean(filepath.Join(destDir, file.Name)) //nolint:gosec
if !strings.HasPrefix(destPath, destDir) {
return errors.Errorf("invalid file path: %q", destPath)
}

// Create directory if it doesn't exist
if file.FileInfo().IsDir() {
err := os.MkdirAll(destPath, file.Mode())
Expand All @@ -44,16 +42,7 @@ func UnzipDir(zipReader *zip.Reader, destDir string) error {
if err != nil {
return errors.WithStack(err)
}
// This is probably a little bit aggressive, in that the symlink can
// only be beneath its parent directory, rather than the root of the
// zip file. But it's good enough for now.
symlinkDir := filepath.Dir(destPath)
symlinkPath, err := sanitizeArchivePath(symlinkDir, buf.String())
if err != nil {
return errors.WithStack(err)
}
symlinkPath = strings.TrimPrefix(symlinkPath, symlinkDir+"/")
err = os.Symlink(symlinkPath, destPath)
err = os.Symlink(buf.String(), destPath)
if err != nil {
return errors.WithStack(err)
}
Expand Down Expand Up @@ -149,13 +138,3 @@ func ZipDir(srcDir, destZipFile string) error {
return nil
}))
}

// Sanitize archive file pathing from "G305: Zip Slip vulnerability"
func sanitizeArchivePath(d, t string) (v string, err error) {
v = filepath.Join(d, t)
if strings.HasPrefix(v, filepath.Clean(d)) {
return v, nil
}

return "", fmt.Errorf("%s: %s", "content filepath is tainted", t)
}

0 comments on commit eef7f10

Please sign in to comment.