On Sun, 5 Oct 2025 at 20:48, Samuel Thibault
<[email protected]> wrote:
>
> From: Viktor Kurilko <[email protected]>
>
> This patch adds the ability to map a host unix socket to a guest tcp socket
> when
> using the slirp backend. This feature was added in libslirp version 4.7.0.
>
> A new syntax for unix socket: -hostfwd=unix:hostpath-[guestaddr]:guestport
>
> Signed-off-by: Viktor Kurilko <[email protected]>
> Signed-off-by: Samuel Thibault <[email protected]>
> Message-ID: <[email protected]>
> ---
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)
> + if (is_unix) {
> + if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
> + fail_reason = "Missing - separator";
> + goto fail_syntax;
> + }
> + if (buf[0] == '\0') {
> + fail_reason = "Missing unix socket path";
> + goto fail_syntax;
> + }
> + if (buf[0] != '/') {
> + fail_reason = "unix socket path must be absolute";
> + goto fail_syntax;
> + }
> +
> + size_t path_len = strlen(buf);
> + if (path_len > sizeof(host_addr.un.sun_path) - 1) {
> + fail_reason = "Unix socket path is too long";
> + goto fail_syntax;
> + }
> +
> + 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 ?
> + error_setg_errno(errp, errno, "Failed to unlink '%s'", buf);
> + goto fail_syntax;
> + }
> + }
> + host_addr.un.sun_family = AF_UNIX;
> + memcpy(host_addr.un.sun_path, buf, path_len);
> + host_addr_size = sizeof(host_addr.un);
> + } else
thanks
-- PMM
