On Thu, 23 Oct 2025 at 11:12, Samuel Thibault <[email protected]> wrote: > > Hello, > > Peter Maydell, le jeu. 23 oct. 2025 10:58:10 +0100, a ecrit: > > Coverity worries here about a possible time-of-check-time-of-use > > bug (CID 1641394). This is a heuristic that tends to fire even > > when there's no interesting attack possible, but I don't > > know what this code is doing so I raise it here: > > > > > +#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0) > > > + struct stat st; > > > + if (stat(buf, &st) == 0) { > > > > Coverity notes that we do a check on the filename here > > with stat()... > > > > > + if (!S_ISSOCK(st.st_mode)) { > > > + fail_reason = "file exists and it's not unix socket"; > > > + goto fail_syntax; > > > + } > > > + > > > + if (unlink(buf) < 0) { > > > > ...and then later we do an unlink() if it's a unix socket. > > But Coverity points out that an attacker could change what > > the filename points to between the stat and the unlink, > > causing us to unlink some non-socket file. > > > > Do we care ? > > It is true that an "attacker" could be pointing qemu to a symlink to > a socket, and stat() will follow it, and they can change the symlink > into something else (a symlink to something else, or another type of > file), and we'll unlink() that (which will not follow any symlink, so > just unlink the given path). > > I don't see which harmful scenario we could have here. Either the > attacker has control over the given path, and we'll just unlink it, too > bad for them, or they don't have control over the given path, and they > won't be able to change it to their liking between stat() and unlink().
Yes, I agree. The "only unlink if it was a unix socket" check is essentially a molly-guard against a user passing a wrong filename. If an attacker bypasses the molly-guard this doesn't gain them anything. Similarly, an attacker reinstating something at the file-path after the unlink() but before slirp binds the unix socket is pretty futile as it will just cause the bind to fail. I'll mark the coverity issue as a false-positive. -- PMM
