Skip to content

Commit bd60432

Browse files
guimardclaude
andcommitted
Fix TOCTOU race condition in cache_key.c
Use open() with O_CREAT|O_EXCL|O_NOFOLLOW and mode 0600 instead of fopen() to create the temp file. This prevents: - Symlink attacks (O_NOFOLLOW) - Race conditions where attacker pre-creates the file (O_EXCL) - Permissions set at creation time, not after (mode 0600) Then use fdopen() to get a FILE* for fprintf(). Also check fclose() return value to catch flush errors (ENOSPC, NFS) that would otherwise result in a truncated file being renamed. Reported by CodeQL, improved based on Copilot review. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 8daf677 commit bd60432

1 file changed

Lines changed: 19 additions & 4 deletions

File tree

src/cache_key.c

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,21 @@ static int load_or_generate_instance_id(const char *cache_dir, char *buf, size_t
115115
return -1;
116116
}
117117

118-
f = fopen(temp_path, "w");
118+
/* Use O_CREAT|O_EXCL|O_NOFOLLOW to prevent symlink attacks and races */
119+
int fd = open(temp_path, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, 0600);
120+
if (fd < 0) {
121+
/* File exists (race) or symlink attack - clean up and retry */
122+
unlink(temp_path);
123+
fd = open(temp_path, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, 0600);
124+
if (fd < 0) {
125+
return -1;
126+
}
127+
}
128+
129+
f = fdopen(fd, "w");
119130
if (!f) {
131+
close(fd);
132+
unlink(temp_path);
120133
return -1;
121134
}
122135

@@ -125,10 +138,12 @@ static int load_or_generate_instance_id(const char *cache_dir, char *buf, size_t
125138
unlink(temp_path);
126139
return -1;
127140
}
128-
fclose(f);
129141

130-
/* Set restrictive permissions */
131-
chmod(temp_path, 0600);
142+
/* Check fclose() return - flush errors (ENOSPC, NFS) would be missed otherwise */
143+
if (fclose(f) != 0) {
144+
unlink(temp_path);
145+
return -1;
146+
}
132147

133148
if (rename(temp_path, id_path) != 0) {
134149
unlink(temp_path);

0 commit comments

Comments
 (0)