Avoid TOCTOU (time-of-check, time-of-use) race condition between stat and rename

Question

How to avoid TOCTOU(time-of-check, time-of-use) race condition for race condition between stat and rename for LOGFILE ?

Required to move the log file after its size value exceeds the max size.

result = stat(LOGFILE, & data);
if (result != 0) {
  // stat failed
  // file probably does not exist
} else if (data.st_size > MAX_LOGSIZE) {
  unlink(PREV_LOGFILE);
  (void) rename(LOGFILE, PREV_LOGFILE);
}

open the file and use fstat, so the file you manipulate cannot "change" — OznOg, Jun 27 '19 at 05:28
Why is this even an issue? Are you writing to the log file from a different thread? You should probably be mutexing it then. It isn't going to shrink in any case, so it isn't like a successful check is ever going to be invalidated. — n. m. could be an AI, Jun 27 '19 at 05:40
@n.m yes different thread , its a coverity issue pointing out — Amruth A, Jun 27 '19 at 05:42
@n.m: Filesystem TOCTTOU isn't an issue about races within a single process. It's about an attacker on your system manipulating the filesystem while you're trying to do something, with the goal of making you do something other than what you intended. The usual case is some sort of access control check followed by a write operation: the attacker lets you do the access check on the correct file, then swaps it out with a link to some other file that you can access but the attacker can't. — user3553031, Jun 27 '19 at 06:26
*'yes different thread'* – *really* thread or possibly rather *process*? If different threads within single process, there are easier ways to cover the issues, e. g. all based on an `std::ofstream` object – in any case, you'd need to protect all this with a `std::mutex`. You'd need to anyway, if more than one thread would be writing to the log file... — Aconcagua, Jun 27 '19 at 06:36
@user3553031 "about an attacker on your system" Are we talking about the question being asked, or about filesystem related TOCTTOU in general? I have no intention to discuss the second one because it is evidently off topic. Regarding the question being asked, there is no risk of an attacker manipulating your file size in order to make you switch the logs, because an attacker with the same permissions can easily do more damage directly without bothering with TOCTTOU. — n. m. could be an AI, Jun 27 '19 at 08:29
@n.m: Assume that LOGFILE is '/path/to/file.log' An attacker with write access to /path, but not to /path/to, could change the link to point to some other directory, leading the victim to delete the wrong file. Yes, that's kind of a stretch, but I don't see any more compelling attack scenarios here. TOCTTOU is mostly a concern when access-control and input-validity decisions are being made. — user3553031, Jun 27 '19 at 16:43
@user3553031 TOCTTOU is exploited by injecting a malicious action between the check and the use. In this case this doesn't make any sense. Whatever this particular attacker can do, he can also do before the check and/or after the use. — n. m. could be an AI, Jun 27 '19 at 17:05

user3553031 · Accepted Answer · 2019-06-27T06:15:36.170

The standard way to avoid TOCTTOU on file operations is to open the file once and then do everything that you need through the file descriptor rather than the file name.

However, both renaming and unlinking a file require its path (because they need to know what link to rename or remove), so you can't use that approach here. An alternative might be to copy the file's contents elsewhere and then truncate it to zero bytes, although your scenario with log files probably requires that the operation be atomic, which may be difficult to achieve. Another approach is to require tight access controls on the directory: if an attacker cannot write to the directory, then it cannot play TOCTTOU games with your process. You can use unlinkat and renameat to restrict your paths to a specific directory's file descriptor so that you don't need to worry about the directory itself changing.

Something like this untested code might do the job, assuming a POSIX-like platform:

dirfd = open(LOGDIR, O_DIRECTORY);
// check for failure
res = fstatat(dirfd, LOGFILE, statbuf, AT_SYMLINK_NOFOLLOW);
if ((0 == res) && (S_ISREG(statbuf) && (data.st_size > MAX_LOGSIZE)) {
    unlinkat(dirfd, PREV_LOGFILE, 0);
    renameat(dirfd, LOGFILE, dirfd, PREV_LOGFILE);
}
close(dirfd);

Avoid TOCTOU (time-of-check, time-of-use) race condition between stat and rename

1 Answers1