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 <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go
index 3ee5b59f18aa94d9cf8ff84c78f3e75b06e4f87b..54d1b8f35bc48e3fc58da42d2e01330534395bea 100644 (file)
--- 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 c862e231f7fbdb0b47f9707c31c9a46d71570dc2..1d39824b9bc593755130773bd28fc5b7a702e6a4 100644 (file)
--- 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()