Hi, I didn't see any reply to your comment on this and other patches. Was that handled somewhere else or are those patches considered final?
Thanks, Johannes On Wed, Oct 05, 2016 at 08:56:52AM +0200, Julien Cristau wrote: > Sorry I didn't get a chance to look at these before they went public... > > On Sun, Sep 25, 2016 at 22:50:40 +0200, Matthieu Herrb wrote: > > > From: Tobias Stoeckmann <[email protected]> > > > > v2: FontNames.c return a NULL list whenever a single > > length field from the server is incohent. > > > > Signed-off-by: Tobias Stoeckmann <[email protected]> > > Reviewed-by: Matthieu Herrb <[email protected]> > > --- > > src/FontNames.c | 23 +++++++++++++++++------ > > src/ListExt.c | 12 ++++++++---- > > src/ModMap.c | 3 ++- > > 3 files changed, 27 insertions(+), 11 deletions(-) > > > > diff --git a/src/FontNames.c b/src/FontNames.c > > index 21dcafe..e55f338 100644 > > --- a/src/FontNames.c > > +++ b/src/FontNames.c > > @@ -66,7 +66,7 @@ int *actualCount) /* RETURN */ > > > > if (rep.nFonts) { > > flist = Xmalloc (rep.nFonts * sizeof(char *)); > > - if (rep.length < (INT_MAX >> 2)) { > > + if (rep.length > 0 && rep.length < (INT_MAX >> 2)) { > > rlen = rep.length << 2; > > ch = Xmalloc(rlen + 1); > > /* +1 to leave room for last null-terminator */ > > @@ -93,11 +93,22 @@ int *actualCount) /* RETURN */ > > if (ch + length < chend) { > > flist[i] = ch + 1; /* skip over length */ > > ch += length + 1; /* find next length ... */ > > - length = *(unsigned char *)ch; > > - *ch = '\0'; /* and replace with null-termination */ > > - count++; > > - } else > > - flist[i] = NULL; > > + if (ch <= chend) { > > + length = *(unsigned char *)ch; > > + *ch = '\0'; /* and replace with null-termination */ > > + count++; > > + } else { > > + Xfree(flist); > > + flist = NULL; > > + count = 0; > > + break; > > + } > > + } else { > > + Xfree(flist); > > + flist = NULL; > > + count = 0; > > + break; > > + } > > } > > } > > *actualCount = count; > > I believe these new error paths are missing Xfree(ch). > > > diff --git a/src/ListExt.c b/src/ListExt.c > > index be6b989..0516e45 100644 > > --- a/src/ListExt.c > > +++ b/src/ListExt.c > > @@ -55,7 +55,7 @@ char **XListExtensions( > > > > if (rep.nExtensions) { > > list = Xmalloc (rep.nExtensions * sizeof (char *)); > > - if (rep.length < (INT_MAX >> 2)) { > > + if (rep.length > 0 && rep.length < (INT_MAX >> 2)) { > > rlen = rep.length << 2; > > ch = Xmalloc (rlen + 1); > > /* +1 to leave room for last null-terminator */ > > @@ -80,9 +80,13 @@ char **XListExtensions( > > if (ch + length < chend) { > > list[i] = ch+1; /* skip over length */ > > ch += length + 1; /* find next length ... */ > > - length = *ch; > > - *ch = '\0'; /* and replace with null-termination */ > > - count++; > > + if (ch <= chend) { > > + length = *ch; > > + *ch = '\0'; /* and replace with null-termination */ > > + count++; > > + } else { > > + list[i] = NULL; > > + } > > } else > > list[i] = NULL; > > } > > This might have been more readable by just replacing the previous > if (ch + length < chend) > with > if (ch + length + 1 < chend) > ? > > Cheers, > Julien > _______________________________________________ > xorg-security mailing list > [email protected] > https://lists.x.org/mailman/listinfo/xorg-security Best regards/Gruesse, Johannes -- GPG Key E7C81FA0 EE16 6BCE AD56 E034 BFB3 3ADD 7BF7 29D5 E7C8 1FA0 Subkey fingerprint: 250F 43F5 F7CE 6F1E 9C59 4F95 BC27 DD9D 2CC4 FD66 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
