----- Carlos R. Mafra <[email protected]> a écrit :
> On Sun, 10 Nov 2013 at 17:41:09 +0100, Christophe wrote:
> > From: Christophe CURIS <[email protected]>
> > 
> > It is not only not very efficient, but in present case it also participates
> > in memory fragmentation.
> > 
> > This patch replaces this with a stack allocated buffer with a buffer which
> > is way too large.
> 
> "Too large" might not be enough to someone explicitly wanting to create
> a buffer overflow attack by using a self-compiled app with a large
> class name, no?
> 
> I'm not too paranoid about this, but it looks like this patch makes the
> code vulnerable for little benefit...

In this current case, "too large" should be interpreted as "I am ashamed to 
take so much space for something that's never gonna be that big" because if 
this gets too long it will be truncated on display anyway.

For the security concern, the code used and still uses s*n*printf, so all that 
can happen is a truncated string, not a buffer overflow.

Hope I have reassured you?


> > Signed-off-by: Christophe CURIS <[email protected]>
> > ---
> >  src/winspector.c | 16 +++++-----------
> >  1 file changed, 5 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/winspector.c b/src/winspector.c
> > index 8cb49df..1e185f5 100644
> > --- a/src/winspector.c
> > +++ b/src/winspector.c
> > @@ -993,26 +993,20 @@ static void textEditedObserver(void *observerData, 
> > WMNotification *notification)
> >  static void selectSpecification(WMWidget *bPtr, void *data)
> >  {
> >     InspectorPanel *panel = (InspectorPanel *) data;
> > -   char *str;
> > +   char str[256];
> >     WWindow *wwin = panel->inspected;
> > -   int len;
> >  
> >     if (bPtr == panel->defaultRb && (wwin->wm_instance || wwin->wm_class))
> >             WMSetButtonEnabled(panel->applyBtn, False);
> >     else
> >             WMSetButtonEnabled(panel->applyBtn, True);
> >  
> > -   len = 16 + strlen(wwin->wm_instance ? wwin->wm_instance : "?")
> > -       + strlen(wwin->wm_class ? wwin->wm_class : "?");
> > -
> > -   str = wmalloc(len);
> > -
> > -   snprintf(str, len, _("Inspecting  %s.%s"),
> > -            wwin->wm_instance ? wwin->wm_instance : "?", wwin->wm_class ? 
> > wwin->wm_class : "?");
> > +   snprintf(str, sizeof(str),
> > +            _("Inspecting  %s.%s"),
> > +            wwin->wm_instance ? wwin->wm_instance : "?",
> > +            wwin->wm_class ? wwin->wm_class : "?");
> >  
> >     wFrameWindowChangeTitle(panel->frame->frame, str);
> > -
> > -   wfree(str);
> >  }
> >  
> >  static void selectWindow(WMWidget *bPtr, void *data)
> > -- 
> > 1.8.4.rc3
> > 
> > 
> > -- 
> > To unsubscribe, send mail to [email protected].
> 
> 
> -- 
> To unsubscribe, send mail to [email protected].


--
To unsubscribe, send mail to [email protected].

Reply via email to