From 0fe0390a2f0cd7da5ace0f45286b1abdb4e7cb95 Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Mon, 26 Jan 2026 09:51:59 +0100 Subject: [PATCH] hs: suppress TOCTOU stat use To explain a bit more the TOCTOU issue found, we can consider a case where Suricata starts to prune, yet externally somebody also starts erasing cache files. Right after Suricata checks the file age with the stat function, somebody may delete or update the file of our interest. Suricata aging decision doesn't reflect the actual state of the file. This commit additionally adds a check for noent failure of the unlink operation (considered as a success). The code can still delete a file that is recently updated but was considered stale. In the documentation-following deployments this should not happen anyway as one cache folder should only be used by a single Suricata instance (and within Suricata instance only one thread handles cache eviction). Additionally, the `stat` and `unlink` command are immediatelly followed, making this scenario extra unlikely. Additional comment in the code explains problems of using fstat and potential issues on Windows. Ticket: 8243 --- src/util-mpm-hs-cache.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/util-mpm-hs-cache.c b/src/util-mpm-hs-cache.c index 58a2aa6ab4..8bdfdfe20a 100644 --- a/src/util-mpm-hs-cache.c +++ b/src/util-mpm-hs-cache.c @@ -374,7 +374,15 @@ int SCHSCachePruneEvaluate(MpmConfig *mpm_conf, HashTable *inuse_caches) continue; struct stat st; - if (stat(path, &st) != 0 || !S_ISREG(st.st_mode)) + /* TOCTOU: race window between stat and unlink is acceptable here. + * On Linux somebody can still modify (use the cache file) between the + * fstat and unlink, on Windows (HS not supported there but still relevant) + * TOCTOU happens when closing the file descriptor and unlinking the file. + * Cache mechanism is best-effort and e.g. not pruning or pruning an extra + * cache file is not problematic. + * Stat is used here to ease file handling as fstat doesn't bring any benefit */ + /* coverity[toctou] */ + if (SCStatFn(path, &st) != 0 || !S_ISREG(st.st_mode)) continue; considered++; @@ -388,7 +396,9 @@ int SCHSCachePruneEvaluate(MpmConfig *mpm_conf, HashTable *inuse_caches) if (cache_inuse != NULL) continue; // in use - if (unlink(path) == 0) { + /* coverity[toctou] */ + int ret = unlink(path); + if (ret == 0 || (ret == -1 && errno == ENOENT)) { removed++; SCLogDebug("File %s removed because of %s%s%s", path, prune_by_age ? "age" : "", prune_by_age && prune_by_version ? " and " : "",