Skip to content

Commit 0ce2428

Browse files
committed
Harden checksum writes against symlink attacks
writeChecksum used os.WriteFile, which follows symlinks. A local attacker who could create a symlink in .gogo/checksum/ before the user ran a task could redirect the write to an arbitrary file. Remove any pre-existing entry first (Remove does not follow symlinks) and then create the file with O_CREATE|O_EXCL so a racing symlink replacement fails the open.
1 parent ad33684 commit 0ce2428

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

taskfile/checksum.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package taskfile
33
import (
44
"crypto/sha256"
55
"encoding/hex"
6+
"errors"
67
"fmt"
78
"io/fs"
89
"os"
@@ -150,13 +151,31 @@ func readStoredChecksum(fileDir, taskName string) string {
150151
return string(data)
151152
}
152153

153-
// writeChecksum stores the checksum for a task.
154+
// writeChecksum stores the checksum for a task. The write is hardened
155+
// against local symlink attacks: we remove any pre-existing entry (without
156+
// following symlinks) and then re-create the file with O_CREATE|O_EXCL so
157+
// a pre-placed symlink in .gogo/checksum/ can't redirect the write
158+
// elsewhere on disk — if an attacker wins the race and recreates a
159+
// symlink between the Remove and the OpenFile, the OpenFile call fails.
154160
func writeChecksum(fileDir, taskName, checksum string) error {
155161
p := checksumPath(fileDir, taskName)
156162
if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
157163
return err
158164
}
159-
return os.WriteFile(p, []byte(checksum), 0o644)
165+
// os.Remove does not follow symlinks, so we only ever delete the entry
166+
// itself, never its target.
167+
if err := os.Remove(p); err != nil && !errors.Is(err, os.ErrNotExist) {
168+
return err
169+
}
170+
f, err := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
171+
if err != nil {
172+
return err
173+
}
174+
if _, err := f.WriteString(checksum); err != nil {
175+
_ = f.Close()
176+
return err
177+
}
178+
return f.Close()
160179
}
161180

162181
// outputsNewerThanSources checks if all generated files exist and are newer

taskfile/checksum_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,35 @@ func TestChecksumPathNoCollision(t *testing.T) {
169169
assert.Equal(t, "underscore", readStoredChecksum(dir, "a_b"))
170170
}
171171

172+
func TestWriteChecksumDoesNotFollowSymlink(t *testing.T) {
173+
// A pre-placed symlink in .gogo/checksum/ must not redirect the write
174+
// to another file on disk: an attacker with write access to .gogo/
175+
// could otherwise have gogo overwrite arbitrary files when the user
176+
// next runs a task.
177+
dir := t.TempDir()
178+
target := filepath.Join(dir, "target.txt")
179+
require.NoError(t, os.WriteFile(target, []byte("original"), 0o644))
180+
181+
require.NoError(t, os.MkdirAll(filepath.Join(dir, ".gogo", "checksum"), 0o755))
182+
link := filepath.Join(dir, ".gogo", "checksum", "build")
183+
require.NoError(t, os.Symlink(target, link))
184+
185+
require.NoError(t, writeChecksum(dir, "build", "new-checksum"))
186+
187+
// The symlink target must remain untouched.
188+
data, err := os.ReadFile(target)
189+
require.NoError(t, err)
190+
assert.Equal(t, "original", string(data), "symlink target must not be overwritten")
191+
192+
// And the cache file must contain the new value.
193+
assert.Equal(t, "new-checksum", readStoredChecksum(dir, "build"))
194+
195+
// The cache entry must be a regular file, not a symlink anymore.
196+
info, err := os.Lstat(link)
197+
require.NoError(t, err)
198+
assert.Zero(t, info.Mode()&os.ModeSymlink, "checksum entry must not be a symlink")
199+
}
200+
172201
func TestOutputsNewerThanSources(t *testing.T) {
173202
dir := t.TempDir()
174203
src := filepath.Join(dir, "main.go")

0 commit comments

Comments
 (0)