1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-07-13 09:07:33 -05:00

Fix memory corruption in scrollback compression.

Introduced by commit 5891142aee, in which I invented
strbuf_shrink_to() and went round replacing lots of assignments of the
form 'sb->len = smaller_value' with calls to it. The bug is also in
0.74, because 34a0460f05 is a cherry-pick of the commit that
introduced it.

The difference between that assignment and strbuf_shrink_to is partly
that the latter checks by assertion that the new length really is
_smaller_ - it doesn't let you accidentally grow a strbuf's length
field beyond the limit of its buffer (or indeed at all). But also,
strbuf_shrink_to re-establishes the strbuf invariant that the text
logically in the buffer is always followed by a zero byte, so that
it's a valid C string.

Unfortunately, in one of the places I made this change, I was storing
binary data in the strbuf (so the terminating NUL is unimportant), and
immediately after decreasing the strbuf's length, I was doing a memcmp
one of whose arguments was the data I'd just chopped off the end of
the strbuf. So it _mattered_ that no random NUL had been splurged over
it.

Specifically, this happened in the run-length encoder used to compress
scrollback data, and had the effect that two components of the
compressed scrollback could be spuriously considered equal, if one of
them started with a legitimate zero byte and the other had a zero byte
written over it by this bug. Thanks to Michael Weller for a nice test
case that demonstrated a compressed scrollback line being decompressed
again as the wrong thing:

"NORMAL TEXT, \033[42mGREEN BACKGROUND\033[0m, NORMAL TEXT AGAIN"

If the above line is printed to the terminal (after being decoded as
if it was a C string literal), then only the words "GREEN BACKGROUND"
get a green background. But after that line is scrolled off the top of
the window, if you find it in the scrollback, then the rest of the
line to the right has also become green-backgrounded due to this bug.
This commit is contained in:
Simon Tatham
2020-10-27 18:26:06 +00:00
parent 150089ac16
commit 9ea0dfd8c0

View File

@ -461,9 +461,10 @@ static void makerle(strbuf *b, termline *ldata,
oldstate = state;
makeliteral(b, c, &state);
tmplen = b->len - tmppos;
bool match = tmplen == thislen &&
!memcmp(b->u + runpos+1, b->u + tmppos, tmplen);
strbuf_shrink_to(b, tmppos);
if (tmplen != thislen ||
memcmp(b->u + runpos+1, b->u + tmppos, tmplen)) {
if (!match) {
state = oldstate;
break; /* run over */
}