[PATCH v2] xclipboard: avoid overflow crash when building labels

[Kees Cook]

Hi, any comments on this?

Thanks,

-Kees

On Sat, Jul 09, 2011 at 07:40:23AM -0700, Kees Cook wrote:
> This replaces sprintf with XtAsprintf to avoid crashing when creating
> various potentially large labels.
> 
> https://bugs.launchpad.net/ubuntu/+source/x11-apps/+bug/792642
> 
> Signed-off-by: Kees Cook <kees.cook at canonical.com>
> ---
>  xclipboard.c |    5 +++--
>  xcutsel.c    |    8 +++++---
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xclipboard.c b/xclipboard.c
> index 1fddf4c..62a214c 100644
> --- a/xclipboard.c
> +++ b/xclipboard.c
> @@ -332,13 +332,14 @@ AcceptSaveFile(Widget w, XEvent *e, String *argv, Cardinal *argc)
>      XtPopdown (fileDialogShell);
>      if (!success)
>      {
> -	char	failMessage[1024];
> +	char	*failMessage;
>  
> -	sprintf (failMessage, "Can't open file \"%s\"", filename);
> +	XtAsprintf (&failMessage, "Can't open file \"%s\"", filename);
>  	XtSetArg (args[0], XtNlabel, failMessage);
>  	XtSetValues (failDialog, args, 1);
>  	CenterWidgetOnEvent (failDialogShell, e);
>  	XtPopup (failDialogShell, XtGrabNone);
> +	XtFree (failMessage);
>      }
>      else
>      {
> diff --git a/xcutsel.c b/xcutsel.c
> index 690e201..3386b57 100644
> --- a/xcutsel.c
> +++ b/xcutsel.c
> @@ -258,7 +258,7 @@ GetBuffer(Widget w, XtPointer closure, XtPointer callData)
>  int 
>  main(int argc, char *argv[])
>  {
> -    char label[100];
> +    char *label;
>      Widget box, button;
>      XtAppContext appcon;
>      Widget shell;
> @@ -288,19 +288,21 @@ main(int argc, char *argv[])
>  	XtAddCallback( button, XtNcallback, Quit, NULL );
>  
>      /* %%% hack alert... */
> -    sprintf(label, "*label:copy %s to %d",
> +    XtAsprintf(&label, "*label:copy %s to %d",
>  	    options.selection_name,
>  	    options.buffer);
>      XrmPutLineResource( &rdb, label );
> +    XtFree(label);
>  
>      button =
>  	XtCreateManagedWidget("sel-cut", commandWidgetClass, box, NULL, ZERO);
>  	XtAddCallback( button, XtNcallback, GetSelection, NULL );
>  
> -    sprintf(label, "*label:copy %d to %s",
> +    XtAsprintf(&label, "*label:copy %d to %s",
>  	    options.buffer,
>  	    options.selection_name);
>      XrmPutLineResource( &rdb, label );
> +    XtFree(label);
>  
>      button =
>  	XtCreateManagedWidget("cut-sel", commandWidgetClass, box, NULL, ZERO);
> -- 
> 1.7.4.1
> 
> 
> 
> -- 
> Kees Cook
> Ubuntu Security Team
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
-- 
Kees Cook
Ubuntu Security Team