Hi all,
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 <tob...@stoeckmann.org>
>
> v2: FontNames.c return a NULL list whenever a single
> length field from the server is incohent.
>
> Signed-off-by: Tobias Stoeckmann <tob...@stoeckmann.org>
> Reviewed-by: Matthieu Herrb <matth...@herrb.eu>
> ---
> 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-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
