On 2021-03-06 02:49 +0530, Utkarsh Gupta wrote: > Hi Thorsten > > On Sat, Mar 6, 2021 at 2:25 AM Thorsten Glaser <t...@mirbsd.de> wrote: >> debian/patches/CVE-2021-27135.patch changes button.c line (after >> patching) 3747 to: >> >> line = realloc(line, screen->selection_size); >> >> But “line” is a local variable, the address of the buffer must >> be stored in the one handed out, too, so please change this to: >> >> if ((have * 2) < (size_t) j) { >> Char *next = realloc(line, have + 1); >> if (next) { >> screen->selection_data = line = next; >> screen->selection_size = have + 1; >> } >> } >> >> This also deals properly with realloc failures (since we’re >> shrinking, ignore them and just keep the older, larger area).
I see that this might be a problem (albeit unlikely to happen in practice), however I have trouble understanding exactly where a use-after-realloc bug comes into play. Maybe Thorsten can help me fix my blindness? > Thanks for the very comprehensive bug report and for the patch as well! > >> I’ve not looked at jessie-ELTS or buster-security whether they >> are affected as well; sid is clean (and where I got the realloc >> failure check necessity from, although sid’s free()s the buffer >> if realloc fails; this isn’t needed @Tom). > > If this seems to be happening in stretch, I assume there's a problem > with jessie-ELTS as well. That said, buster is good because a DSA > wasn't issued and this will be fixed via a point release. I had already prepared an update for buster, but fortunately it did not happen yet, because that one also has the same bug as yours. > I am glad and surprised that sid is okay and there doesn't seem to be > a problem. Just to cross-check and ensure I get it right (for stretch > and jessie), can you send me the reproducer as well? I'd like to be > able to reproduce this before and after your patch (just to be one the > safer side) and do the same for jessie as well! Run xterm under valgrind and select some text. Valgrind will be very unhappy with xterm 327-2+deb9u1 but should not show up any errors in that regard with a correctly patched version. Instead of Thorsten's suggestion you could also apply the patches to the SaltTextAway() function from xterm 365e. Cheers, Sven