|
From: JVZ <je...@fo...> - 2016-04-05 14:07:03
|
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. | +----------------------------------------------------------------------------+ |