Skip to content

Commit

Permalink
glog: have createInDir fail if the file already exists
Browse files Browse the repository at this point in the history
This prevents an attack like the one described
[here](https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File#:~:text=On%20Unix%20based,with%20elevated%20permissions.).
An unprivileged attacker could use symlinks to trick a privileged
logging process to follow a symlink from the log dir and write logs over
an arbitrary file.

The components of the log names are program, host, username, tag, date,
time and PID. These are all predictable. It's not at all unusual for the
logdir to be writable by unprivileged users, and one of the fallback
directories (/tmp) traditionally has broad write privs with the sticky
bit set on Unix systems.

As a concrete example, let's say I've got a glog-enabled binary running
as a root cronjob. I can gauge when that cron job will run and then use
a bash script to spray the log dir with glog-looking symlinks to
`/etc/shadow` with predicted times and PIDs. When the cronjob runs, the
`os.Create` call will follow the symlink, truncate `/etc/shadow` and
then fill it with logs.

This change defeats that by setting `O_EXCL`, which will cause the open
call to fail if the file already exists.

Fixes CVE-2024-45339

cl/712795111 (google-internal)
  • Loading branch information
chressie authored and stapelberg committed Jan 13, 2025
1 parent 7139da2 commit a0e3c40
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
7 changes: 6 additions & 1 deletion glog_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,12 @@ func create(tag string, t time.Time, dir string) (f *os.File, filename string, e
func createInDir(dir, tag string, t time.Time) (f *os.File, name string, err error) {
name, link := logName(tag, t)
fname := filepath.Join(dir, name)
f, err = os.Create(fname)
// O_EXCL is important here, as it prevents a vulnerability. The general idea is that logs often
// live in an insecure directory (like /tmp), so an unprivileged attacker could create fname in
// advance as a symlink to a file the logging process can access, but the attacker cannot. O_EXCL
// fails the open if it already exists, thus prevent our this code from opening the existing file
// the attacker points us to.
f, err = os.OpenFile(fname, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
if err == nil {
symlink := filepath.Join(dir, link)
os.Remove(symlink) // ignore err
Expand Down
11 changes: 11 additions & 0 deletions glog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,3 +772,14 @@ func TestLogLength(t *testing.T) {
len(c), logsink.MaxLogMessageLen, c)
}
}

func TestCreateFailsIfExists(t *testing.T) {
tmp := t.TempDir()
now := time.Now()
if _, _, err := create("INFO", now, tmp); err != nil {
t.Errorf("create() failed on first call: %v", err)
}
if _, _, err := create("INFO", now, tmp); err == nil {
t.Errorf("create() succeeded on second call, want error")
}
}

0 comments on commit a0e3c40

Please sign in to comment.