[PATCH v2 05/25] [libx11] Fixed by using strncpy and explicitly terminating the buffer
Alan Coopersmith
alan.coopersmith at oracle.com
Mon Jan 31 19:30:28 PST 2011
On 01/31/11 04:01 AM, Erkki Seppälä wrote:
> Possible overrun of 8192 byte fixed size buffer "buffer" by copying "ext->name" without length checking
>
> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira at nokia.com>
> Signed-off-by: Erkki Seppälä <erkki.seppala at vincit.fi>
> ---
> src/XlibInt.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/XlibInt.c b/src/XlibInt.c
> index d55c26a..52ccff1 100644
> --- a/src/XlibInt.c
> +++ b/src/XlibInt.c
> @@ -1438,9 +1438,10 @@ static int _XPrintDefaultError(
> ext && (ext->codes.major_opcode != event->request_code);
> ext = ext->next)
> ;
> - if (ext)
> - strcpy(buffer, ext->name);
> - else
> + if (ext) {
> + strncpy(buffer, ext->name, BUFSIZ);
> + buffer[BUFSIZ - 1] = '\0';
> + } else
> buffer[0] = '\0';
> }
> (void) fprintf(fp, " (%s)\n", buffer);
If we ever have an extension with a name > 8192 characters, someone needs to be
hurt. Unfortunately, static analysis tools can't assume we have common sense
(X clearly being full of counter examples), and the error printing code is never
going to be a performance bottleneck, so may as well fix to reduce the noise:
Reviewed-by: Alan Coopersmith <alan.coopersmith at oracle.com>
--
-Alan Coopersmith- alan.coopersmith at oracle.com
Oracle Solaris Platform Engineering: X Window System
More information about the xorg-devel
mailing list