Re: cabinet: Fix NULL pointer dereference (Coverity)

[James Hawkins]

On 9/26/06, Andrew Talbot <Andrew.Talbot(a)talbotville.com> wrote:
> Changelog:
>     cabinet: Fix NULL pointer dereference (Coverity).
>
> diff -urN a/dlls/cabinet/fci.c b/dlls/cabinet/fci.c
> --- a/dlls/cabinet/fci.c        2006-09-12 23:59:28.000000000 +0100
> +++ b/dlls/cabinet/fci.c        2006-09-26 18:27:03.000000000 +0100
> @@ -176,18 +176,22 @@
>    if ((!perf) || (!pfnalloc) || (!pfnfree) || (!pfnopen) || (!pfnread) ||
>        (!pfnwrite) || (!pfnclose) || (!pfnseek) || (!pfndelete) ||
>        (!pfnfcigtf) || (!pccab)) {
> -    perf->erfOper = FCIERR_NONE;
> -    perf->erfType = ERROR_BAD_ARGUMENTS;
> -    perf->fError = TRUE;
> +    if (perf) {
> +      perf->erfOper = FCIERR_NONE;
> +      perf->erfType = ERROR_BAD_ARGUMENTS;
> +      perf->fError = TRUE;
> +    }
>
> SetLastError(ERROR_BAD_ARGUMENTS);
>      return NULL;
>    }
>
> if (!((hfci = ((HFCI) (*pfnalloc)(sizeof(FCI_Int)))))) {
> -    perf->erfOper = FCIERR_ALLOC_FAIL;
> -    perf->erfType = ERROR_NOT_ENOUGH_MEMORY;
> -    perf->fError = TRUE;
> +    if (perf) {
> +      perf->erfOper = FCIERR_ALLOC_FAIL;
> +      perf->erfType = ERROR_NOT_ENOUGH_MEMORY;
> +      perf->fError = TRUE;
> +    }
>
> SetLastError(ERROR_NOT_ENOUGH_MEMORY);
>      return NULL;

This last check is pointless.  If perf is NULL, we bail out and return
NULL a few lines up.  Also, please add a test showing that native does
not crash if a NULL perf is sent in.  It's weird, but we need to be
consistent with native, as some apps depend on such a crash.

-- 
James Hawkins

cabinet: Fix NULL pointer dereference (Coverity)

[Andrew Talbot]

> This last check is pointless.  If perf is NULL, we bail out and return
> NULL a few lines up.  Also, please add a test showing that native does
> not crash if a NULL perf is sent in.  It's weird, but we need to be
> consistent with native, as some apps depend on such a crash.

Whoops! Thanks, James; I was a bit lax, there.

-- Andy.