From 76811213d7f927c15220c91b4c47af7028af4f47 Mon Sep 17 00:00:00 2001 From: Mohit Agarwal Date: Tue, 3 Nov 2015 00:04:46 +0530 Subject: [PATCH] cmd/go: check if destination is a regular file builder.copyFile ensures that the destination is an object file. This wouldn't be true if we are not writing to a regular file and the copy fails. Check if the destination is an object file only if we are writing to a regular file. While removing the file, ensure that it is a regular file so that device files and such aren't removed when running as a user with suggicient privileges. Fixes #12407 Change-Id: Ie86ce9770fa59aa56fc486a5962287859b69db3d Reviewed-on: https://go-review.googlesource.com/16585 Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/cmd/go/build.go | 16 +++++++++++++--- src/cmd/go/go_test.go | 8 ++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go index 3ee5b59f18..54d1b8f35b 100644 --- a/src/cmd/go/build.go +++ b/src/cmd/go/build.go @@ -1669,7 +1669,7 @@ func (b *builder) copyFile(a *action, dst, src string, perm os.FileMode, force b if fi.IsDir() { return fmt.Errorf("build output %q already exists and is a directory", dst) } - if !force && !isObject(dst) { + if !force && fi.Mode().IsRegular() && !isObject(dst) { return fmt.Errorf("build output %q already exists and is not an object file", dst) } } @@ -1681,7 +1681,7 @@ func (b *builder) copyFile(a *action, dst, src string, perm os.FileMode, force b } } - os.Remove(dst) + mayberemovefile(dst) df, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm) if err != nil && toolIsWindows { // Windows does not allow deletion of a binary file @@ -1700,7 +1700,7 @@ func (b *builder) copyFile(a *action, dst, src string, perm os.FileMode, force b _, err = io.Copy(df, sf) df.Close() if err != nil { - os.Remove(dst) + mayberemovefile(dst) return fmt.Errorf("copying %s to %s: %v", src, dst, err) } return nil @@ -1765,6 +1765,16 @@ func isObject(s string) bool { return false } +// mayberemovefile removes a file only if it is a regular file +// When running as a user with sufficient privileges, we may delete +// even device files, for example, which is not intended. +func mayberemovefile(s string) { + if fi, err := os.Lstat(s); err == nil && !fi.Mode().IsRegular() { + return + } + os.Remove(s) +} + // fmtcmd formats a command in the manner of fmt.Sprintf but also: // // If dir is non-empty and the script is not in dir right now, diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index c862e231f7..1d39824b9b 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1165,6 +1165,14 @@ func TestInstallIntoGOPATH(t *testing.T) { tg.wantExecutable("testdata/bin/go-cmd-test"+exeSuffix, "go install go-cmd-test did not write to testdata/bin/go-cmd-test") } +// Issue 12407 +func TestBuildOutputToDevNull(t *testing.T) { + tg := testgo(t) + defer tg.cleanup() + tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata")) + tg.run("build", "-o", os.DevNull, "go-cmd-test") +} + func TestPackageMainTestImportsArchiveNotBinary(t *testing.T) { tg := testgo(t) defer tg.cleanup() -- 2.48.1