upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-05-28 04:34:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Stop isc_file_safecreate from following symlinks

Browse source 
The function existence-checked the target with stat() and then opened
the same path without O_NOFOLLOW, so a symlink at the target path
passed the regular-file test against the link's destination and the
open() that followed truncated and wrote through the link.
rndc-confgen -a is typically run as root and writes the keyfile under
a directory that service accounts may have write access to, so a stray
symlink there would silently redirect the truncate, fchown, and
overwrite to whatever file the link pointed at.

Switch the existence check to lstat() and use S_ISREG() so a symlink's
S_IFLNK mode is detected directly (a plain bitmask of S_IFREG matches
both, since S_IFLNK shares its high bit). Add O_NOFOLLOW to both
open() flag sets to close the lstat/open TOCTOU window. Hardening
against unexpected symlinks on intermediate path components is out of
scope.

Assisted-by: Claude:claude-opus-4-7
This commit is contained in:
Ondřej Surý 2026-04-29 15:30:05 +02:00
parent 5b164b551f
commit 6082274450
3 changed files with 50 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
lib/isc/file.c
View file

@ -91,6 +91,20 @@ file_stats(const char *file, struct stat *stats) {
return result;
}
static isc_result_t
file_lstats(const char *file, struct stat *stats) {
isc_result_t result = ISC_R_SUCCESS;
REQUIRE(file != NULL);
REQUIRE(stats != NULL);
if (lstat(file, stats) != 0) {
result = isc__errno2result(errno);
}
return result;
}
static isc_result_t
fd_stats(int fd, struct stat *stats) {
isc_result_t result = ISC_R_SUCCESS;
@ -607,14 +621,14 @@ isc_file_safecreate(const char *filename, FILE **fp) {
REQUIRE(filename != NULL);
REQUIRE(fp != NULL && *fp == NULL);
result = file_stats(filename, &sb);
result = file_lstats(filename, &sb);
if (result == ISC_R_SUCCESS) {
if ((sb.st_mode & S_IFREG) == 0) {
if (!S_ISREG(sb.st_mode)) {
return ISC_R_INVALIDFILE;
}
flags = O_WRONLY | O_TRUNC;
flags = O_WRONLY | O_TRUNC | O_NOFOLLOW;
} else if (result == ISC_R_FILENOTFOUND) {
flags = O_WRONLY | O_CREAT | O_EXCL;
flags = O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW;
} else {
return result;
}

5
lib/isc/include/isc/file.h
View file

@ -288,8 +288,9 @@ isc_file_truncate(const char *filename, off_t size);
isc_result_t
isc_file_safecreate(const char *filename, FILE **fp);
/*%<
* Open 'filename' for writing, truncating if necessary. Ensure that
* if it existed it was a normal file. If creating the file, ensure
* Open 'filename' for writing, truncating if necessary. If 'filename'
* exists it must be a regular file; symbolic links (and any other
* non-regular file types) are rejected. If creating the file, ensure
* that only the owner can read/write it.
*/

29
tests/isc/file_test.c
View file

@ -89,6 +89,34 @@ ISC_RUN_TEST_IMPL(isc_file_sanitize) {
unlink(F(SHA));
}
/* test that isc_file_safecreate refuses to follow symlinks */
ISC_RUN_TEST_IMPL(isc_file_safecreate_symlink) {
char target[] = "safecreate_target.XXXXXX";
char link[] = "safecreate_link.XXXXXX";
int fd;
FILE *fp = NULL;
isc_result_t result;
UNUSED(state);
fd = mkstemp(target);
assert_int_not_equal(fd, -1);
close(fd);
fd = mkstemp(link);
assert_int_not_equal(fd, -1);
close(fd);
unlink(link);
assert_return_code(symlink(target, link), 0);
result = isc_file_safecreate(link, &fp);
assert_int_equal(result, ISC_R_INVALIDFILE);
assert_null(fp);
unlink(link);
unlink(target);
}
/* test filename templates */
ISC_RUN_TEST_IMPL(isc_file_template) {
isc_result_t result;
@ -135,6 +163,7 @@ ISC_RUN_TEST_IMPL(isc_file_template) {
ISC_TEST_LIST_START
ISC_TEST_ENTRY(isc_file_sanitize)
ISC_TEST_ENTRY(isc_file_safecreate_symlink)
ISC_TEST_ENTRY(isc_file_template)
ISC_TEST_LIST_END