On 251014 0030, Navid Emamdoost wrote:
> Add a new generic fuzz target for the QEMU VNC server. This target
> exercises both the standard VNC protocol and the VNC-over-WebSocket
> transport layer, increasing coverage of a primary remote attack surface.
>
> To support parallel fuzzing (e.g., with oss-fuzz), the VNC unix
> socket paths are generated dynamically. The fuzzer harness inspects the
> command line for placeholders and replaces them with unique paths
> created by mkstemp() before execution.
>
> ---
>
> This new target increases code coverage in the VNC subsystem
> and related networking and I/O code.
> The baseline coverage below was generated by running all existing fuzz
> targets with the oss-fuzz corpus. The new target shows significant gains:
>
> ----------------------------------------------------------------------------
> File                       New Target                Baseline        Change
> ----------------------------------------------------------------------------
> vnc.c                      339/3212 (10.6%)     3/3212 (0.1%)        +336
> keymaps.c                  91/184 (49.5%)       0/184 (0.0%)         +91
> net-listener.c             76/198 (38.4%)       3/198 (1.5%)         +73
> channel-socket.c           73/575 (12.7%)       19/575 (3.3%)        +54
> qemu-sockets.c             44/1019 (4.3%)       0/1019 (0.0%)        +44
> vnc-jobs.c                 41/219 (18.7%)       0/219 (0.0%)         +41
> dns-resolver.c             28/145 (19.3%)       3/145 (2.1%)         +25
>
> Signed-off-by: Navid Emamdoost <[email protected]>
> ---
>  tests/qtest/fuzz/fuzz.c                 | 65 +++++++++++++++++++++++++
>  tests/qtest/fuzz/generic_fuzz_configs.h |  5 ++
>  2 files changed, 70 insertions(+)
>
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index ca248a51a6..b77c3ceb2b 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -126,6 +126,46 @@ static FuzzTarget *fuzz_get_target(char* name)
>      return NULL;
>  }
>
> +/*
> + * Global variables to hold the unique socket paths for cleanup.
> + */
> +static char g_vnc_socket_path[sizeof("/tmp/qemu-vnc.XXXXXX")];
> +static char g_vnc_ws_socket_path[sizeof("/tmp/qemu-vnc-ws.XXXXXX")];
> +
> +/*
> + * atexit() handler to clean up both socket files.
> + */
> +static void cleanup_vnc_sockets(void)
> +{
> +    if (g_vnc_socket_path[0] != '\0') {
> +        unlink(g_vnc_socket_path);
> +    }
> +    if (g_vnc_ws_socket_path[0] != '\0') {
> +        unlink(g_vnc_ws_socket_path);
> +    }
> +}
> +
> +/* Helper function to find and replace a placeholder in a GString */
> +static bool replace_socket_placeholder(GString *cmd_line, const char 
> *placeholder,
> +                                       const char *path_template, char 
> *global_path_out)
> +{
> +    char *placeholder_ptr = strstr(cmd_line->str, placeholder);
> +    if (placeholder_ptr) {
> +        int fd;
> +        strcpy(global_path_out, path_template);
> +        fd = mkstemp(global_path_out);
Appologies for the delay. Let's use g_mkstemp, if possible.

> +        if (fd == -1) {
> +            perror("mkstemp failed");
> +            return false;
> +        }
> +        close(fd);
> +
> +        gssize pos = placeholder_ptr - cmd_line->str;
> +        g_string_erase(cmd_line, pos, strlen(placeholder));
> +        g_string_insert(cmd_line, pos, global_path_out);
> +    }
> +    return true;
> +}
>
>  /* Sometimes called by libfuzzer to mutate two inputs into one */
>  size_t LLVMFuzzerCustomCrossOver(const uint8_t *data1, size_t size1,
> @@ -213,6 +253,31 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char 
> ***envp)
>      g_string_append_printf(cmd_line, " %s -qtest /dev/null ",
>                             getenv("QTEST_LOG") ? "" : "-qtest-log none");
>
> +    /*
> +     * For the VNC fuzzer, we replace placeholders for both the standard
> +     * and WebSocket VNC listeners with unique socket paths.
> +     */
> +    if (strcmp(fuzz_target->name, "generic-fuzz-vnc") == 0) {

Can generic_fuzz_config.argfunc be used here, instead?
We use  it for virtio_9p to create/share temp directories in
generic_fuzzer_virtio_9p_args. I would rather have one way of
dynamically generating arguments.

-Alex

> +        bool success = true;
> +        success &= replace_socket_placeholder(cmd_line, "VNC_SOCKET_PATH",
> +                                              "/tmp/qemu-vnc.XXXXXX", 
> g_vnc_socket_path);
> +        success &= replace_socket_placeholder(cmd_line, "VNC_WS_SOCKET_PATH",
> +                                              "/tmp/qemu-vnc-ws.XXXXXX", 
> g_vnc_ws_socket_path);
> +
> +        if (!success) {
> +            exit(1);
> +        }
> +
> +        /* Check that placeholders were actually found and replaced */
> +        if (g_vnc_socket_path[0] == '\0' || g_vnc_ws_socket_path[0] == '\0') 
> {
> +            fprintf(stderr, "ERROR: VNC fuzzer is missing a socket 
> placeholder\n");
> +            exit(1);
> +        }
> +
> +        /* Register a single cleanup handler for both sockets */
> +        atexit(cleanup_vnc_sockets);
> +    }
> +



>      /* Split the runcmd into an argv and argc */
>      wordexp_t result;
>      wordexp(cmd_line->str, &result, 0);
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
> b/tests/qtest/fuzz/generic_fuzz_configs.h
> index ef0ad95712..bd2d875dd8 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -247,6 +247,11 @@ const generic_fuzz_config predefined_configs[] = {
>          .args = "-machine q35 -nodefaults "
>          "-parallel file:/dev/null",
>          .objects = "parallel*",
> +    },{
> +        .name = "vnc",
> +        .args = "-machine q35 -nodefaults "
> +             "-vnc 
> vnc=unix:VNC_SOCKET_PATH,websocket=unix:VNC_WS_SOCKET_PATH",
> +        .objects = "*",
>      }
>  };
>
> --
> 2.51.0.760.g7b8bcc2412-goog
>

Reply via email to