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

Huw Davies huw at codeweavers.com
Thu May 17 02:50:11 CDT 2018


On Wed, May 16, 2018 at 11:29:21AM -0500, Vincent Povirk wrote:
> Signed-off-by: Vincent Povirk <vincent at codeweavers.com>
> ---
>  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.



More information about the wine-devel mailing list