Re: [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

On Tue, Sep 09, 2014 at 09:45:06AM +0200, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> It was possible to call strcmp with NULL argument, that can cause
> segmentation fault. Properly checking parameters to prevent this
> situation.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  util/uri.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/util/uri.c b/util/uri.c
> index e348c17..16c01d0 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -1985,7 +1985,8 @@ uri_resolve_relative (const char *uri, const char * base)
>  	val = g_strdup (uri);
>  	goto done;
>      }
> -    if (!strcmp(bas->path, ref->path)) {
> +    if (bas->path != NULL && ref->path != NULL && 
> +	 !strcmp(bas->path, ref->path)) {
>  	val = g_strdup("");
>  	goto done;
>      }

This patch adds more conditions, what's needed is to audit and clean up this
function.  There is dead code and things could be simplified instead:

    if (!strcmp(bas->path, ref->path)) {
        val = g_strdup("");
        goto done;
    }
    if (bas->path == NULL) {
        val = g_strdup(ref->path);
        goto done;
    }
    if (ref->path == NULL) {
        ref->path = (char *) "/";
        remove_path = 1;
    }
    <---- bas->path cannot be NULL because we took goto done
    <---- ref->path cannot be NULL because we assigned "/"

    /*
     * At this point (at last!) we can compare the two paths
     *
     * First we take care of the special case where either of the
     * two path components may be missing (bug 316224)
     */
    if (bas->path == NULL) {  <--- dead code!
        if (ref->path != NULL) {
            uptr = ref->path;
            if (*uptr == '/')
                uptr++;
            /* exception characters from uri_to_string */
            val = uri_string_escape(uptr, "/;&=+$,");
        }
        goto done;
    }
    bptr = bas->path;
    if (ref->path == NULL) {  <--- dead code!

Also, perhaps the strcmp() you touched should really be moved below the NULL
checks.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]