Fixes from self-review

spikecurtis · spikecurtis · commit 39d606537bdb · 2023-08-23T09:51:48.000Z
Signed-off-by: Spike Curtis &lt;spike@coder.com&gt;
diff --git a/provisioner/echo/serve.go b/provisioner/echo/serve.go
@@ -188,7 +188,6 @@ func (e *echo) Apply(sess *provisionersdk.Session, req *proto.ApplyRequest, canc
 
 	// some tests use Echo without a complete response to test cancel
 	<-canceledOrComplete
-	// we have to return a clean Complete or the status will go to "failed"
 	return provisionersdk.ApplyErrorf("canceled")
 }
 
diff --git a/provisionersdk/session.go b/provisionersdk/session.go
@@ -177,8 +177,6 @@ func (s *Session) Context() context.Context {
 func (s *Session) ExtractArchive() error {
 	ctx := s.Context()
 
-	//s.ProvisionLog(proto.LogLevel_INFO, "Setting up")
-
 	s.Logger.Info(ctx, "unpacking template source archive",
 		slog.F("size_bytes", len(s.Config.TemplateSourceArchive)),
 	)
@@ -194,7 +192,11 @@ func (s *Session) ExtractArchive() error {
 			}
 			return xerrors.Errorf("read template source archive: %w", err)
 		}
-		// #nosec
+		// Security: don't untar absolute or relative paths, as this can allow a malicious tar to overwrite
+		// files outside the workdir.
+		if !filepath.IsLocal(header.Name) {
+			return xerrors.Errorf("refusing to extract to non-local path")
+		}
 		headerPath := filepath.Join(s.WorkDirectory, header.Name)
 		if !strings.HasPrefix(headerPath, filepath.Clean(s.WorkDirectory)) {
 			return xerrors.New("tar attempts to target relative upper directory")

Original file line number	Diff line number	Diff line change
`@@ -188,7 +188,6 @@ func (e echo) Apply(sess provisionersdk.Session, req *proto.ApplyRequest, canc`
`188`	`188`
`189`	`189`	`// some tests use Echo without a complete response to test cancel`
`190`	`190`	`<-canceledOrComplete`
`191`		`- // we have to return a clean Complete or the status will go to "failed"`
`192`	`191`	`return provisionersdk.ApplyErrorf("canceled")`
`193`	`192`	`}`
`194`	`193`