[PATCH] wineps.drv: check for GDI_ERROR in LoadTable()

[Wolfgang Walter]

Am Montag, 12. November 2018, 10:57:59 schrieb Huw Davies:
> On Sat, Nov 10, 2018 at 12:33:30AM +0100, Wolfgang Walter wrote:
> > On Friday, 9 November 2018 18:50:43 CET Nikolay Sivov wrote:
> > > On 11/9/18 4:21 PM, Wolfgang Walter wrote:
> > > >       if(table->MS_tag == MS_MAKE_TAG('g','d','i','r')) return TRUE;
> > > >       table->len = GetFontData(hdc, table->MS_tag, 0, NULL, 0);
> > > > 
> > > > +    table->check = 0;
> > > > +    if(table->len == GDI_ERROR) {
> > > > +        table->len = 0;
> > > > +        return TRUE;
> > > > +    }
> > > > +    if(table->len > (0xfffffffflu - 3)) {
> > > > +        table->len = 0;
> > > > +        return FALSE;
> > > > +    }
> > > 
> > > What is the second condition for?
> > 
> > The code which follows is:
> > 
> > table->data = HeapAlloc(GetProcessHeap(), 0, (table->len + 3) & ~3 );
> > memset(table->data + ((table->len - 1) & ~3), 0, sizeof(DWORD));
> > GetFontData(hdc, table->MS_tag, 0, table->data, table->len);
> > for(i = 0; i < (table->len + 3) / 4; i++)
> > 
> >     table->check += FLIP_ORDER(*((DWORD*)(table->data) + i));
> > 
> > If table->len (which itself is a DWORD) gets bigger than 0xfffffffflu - 3
> > it will overflow in (table->len + 3) and HeapAlloc does not allocate as
> > much memory as expected.
> 
> I don't think that's worth covering.  I've sent in a cleaner version.
> 

Wouldn't it possible that someone sends a PDF with a table len of say 
0xfffffffe? Then the allocated memory is small and the later memset writes 0 
beyond. Actually I found this problem with PDF embedded font so if this is not 
checked elsewhere it is scary.

But of one should then also check that len is not to small? It should be at 
least 1 so that table->len - 1 does not underflow.

I don't know if plausibilty of the table lenght is already checked elsewhere, 
of course.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts