Re: [Foxgui-users] Invalid read in FXText::paintCursor (FOX 1.7.54)

[JVZ <je...@fo...>]

On Tue, 5 Apr 2016 11:03:08 +0200
Frank Weimer <fw...@so...> wrote:

>Hi,
>
>I just noticed a problem in the functions
>
>  * void FXText::paintCursor(FXDCWindow& dc) const, and
>  * void FXText::eraseCursor(FXDCWindow& dc) const
>
>of the latest version (1.7.54) of FXText (1.7.53 is still fine).
>
>If the current cursor position is at the end of the buffer, the statement
>
>    tw=font->getCharWidth((*c=getChar(cursorpos)*)>=' '?c:' ');
>
>accesses an invalid memory region (just beyond the allocated buffer).

I can see the issue myself over here, and as far as I can tell the out-of-
bounds read is confined to the cursor-blinking part of FXText.

>The problem can be fixed by using
>
>    tw=font->getCharWidth((*c=cursorpos<length?getChar(cursorpos):'
>    '*)>=' '?c:' ');  
>
>instead.
>
>Maybe it would be even better if the function
>
>    FXwchar FXText::getChar(FXint pos) const
>
>would use a validity check for its parameter (and then return 0 for
>invalid positions)?!

Well, its so heavily used that this might become a serious performance hit.
[Just when I speeded things up a tad with a non-branching trick to handle the
buffer gap].

I *did* add an FXASSERT() in there now so at least a thing like this will be 
discovered when compiled for debug mode, when people don't expect things to
be fast anyway...

Also, scanning through the code, it looks like the cursor paint/erase is
the only place where the buffer position is actually at the end of the
buffer.  Of course, that should not happen as there is no characters there.

A cursor position at the end of the buffer is legal, however.


		-- JVZ


+----------------------------------------------------------------------------+
| Copyright (C) 08:50 04/ 5/2016 Jeroen van der Zijp.   All Rights Reserved. |
+----------------------------------------------------------------------------+