#422 Invalid read in FindHelp() found by valgrind

[Juergen Wieferink]

Ethan has pointed out that my first fix to this in 2657599 is not correct (thanks). This one corrects both the left (i.e. the second) border of the copy loop as well as prevents an invalid write behind the end of the allocated area. I have checked the code path and found that keyword is allocated as MAX_LINE_LEN.
This fix is not really a clean solution, but it is better than the current state.

Juergen

[Juergen Wieferink]

patch

[Ethan Merritt]

Sorry to be so picky, but this fix is still not correct.

It is true that the original buffer was allocated with length MAX_LINE_LEN. But the keyword being queried may in principle start anywhere within this buffer, so we do not know inside the FindHelp() routine how much space remains in the buffer that was passed to us. I think the whole mechanism is flawed. In order for this operation to be safe, the low level routine would have to receive the buffer start, length, and current index rather than, as now, receiving only a pointer to somewhere in the buffer. Better yet, the lower level routine should not mess with the buffer at all. It could instead return a pointer to the full-length keyword and let the calling routine use this to modify its own buffer.

In practice no legitimate help query will involve passing a nearly-full MAX_LINE_LEN buffer. But if we're going to the trouble of fixing possible buffer overflows, let's do a proper job of it.

[Juergen Wieferink]

True. I should not meddle with code paths I'm not familiar with.

I'd propose to either move this item to the Bugs or to close it, then.

Juergen

[Ethan Merritt]

In preparation for eventual release of gnuplot version 4.4, existing bugs and patchsets are being re-prioritized.

I have chosen to use the following categories

9 - should be included in 4.4.rc1 if possible
7 - looks reasonable to me, but not high priority for 4.4.rc1
5 - default priority for newly submitted patches
2 - not under active consideration

These are my personal ratings. If you want to argue for a different priority on one or more patches - go right ahead.

Ethan Merritt (sfeam)