Re: [PATCH 1/3] gdi32: Add bounds checking for metafile records.

[Huw Davies]

On Wed, May 16, 2018 at 11:29:21AM -0500, Vincent Povirk wrote:
> Signed-off-by: Vincent Povirk
>
> dlls/gdi32/enhmetafile.c | 610 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 610 insertions(+)
>
> diff --git a/dlls/gdi32/enhmetafile.c b/dlls/gdi32/enhmetafile.c
> index 2d8a9b04fb2..d8f72c98718 100644
> --- a/dlls/gdi32/enhmetafile.c
> +++ b/dlls/gdi32/enhmetafile.c
> @@ -743,6 +743,604 @@ static BOOL emr_produces_output(int type)
>      }
>  }
>
> +static BOOL emr_validate_size(const ENHMETARECORD *emr)
> +{
> - switch(emr->iType) {
> - case EMR_HEADER:
> -
>  if (emr->nSize < FIELD_OFFSET(ENHMETAHEADER, cbPixelFormat)) return FALSE;
> -
>  break;
> - case EMR_EOF:
> - {
> -
>  const EMREOF *lpEof = (const EMREOF *)emr;
> -
>  if (emr->nSize < sizeof(EMREOF) ||
> -
>      (lpEof->nPalEntries * 4 / 4) != lpEof->nPalEntries ||
> -
>      lpEof->offPalEntries > lpEof->offPalEntries + lpEof->nPalEntries * 4 ||
> -
>      emr->nSize < lpEof->offPalEntries + lpEof->nPalEntries * 4)
> -
>    return FALSE;
> -
>  break;
> - }
> - case EMR_GDICOMMENT:
> - {
> -
>  const EMRGDICOMMENT *lpGdiComment = (const EMRGDICOMMENT *)emr;
> -
>  if (emr->nSize < FIELD_OFFSET(EMRGDICOMMENT, Data) ||
> -
>      lpGdiComment->cbData > lpGdiComment->cbData + FIELD_OFFSET(EMRGDICOMMENT, Data) ||
> -
>      emr->nSize < lpGdiComment->cbData + FIELD_OFFSET(EMRGDICOMMENT, Data))
> -
>    return FALSE;
> -
>  break;
> - }
> - case EMR_SETMAPMODE:
> -
>  if (emr->nSize < sizeof(EMRSETMAPMODE)) return FALSE;
> -
>  break;
> - case EMR_SETBKMODE:
> -
>  if (emr->nSize < sizeof(EMRSETBKMODE)) return FALSE;
> -
>  break;
> - case EMR_SETBKCOLOR:
> -
>  if (emr->nSize < sizeof(EMRSETBKCOLOR)) return FALSE;
> -
>  break;

Since there are so many fixed-size records, it would most likely be
cleaner to use a lookup table, storing zero (or something) for the
variable-size records, then handle those remaining ones in a switch.

Please issue a FIXME() on unhandled record types - I'm ok with a
FIXME comment for partially implemented ones, but unknown record
types should get a FIXME().

Also, please use 4-space indents.

Huw.