Bug#984615: xterm: bug in CVE-2021-27135 patch in at least stretch
On 2021-03-06 02:49 +0530, Utkarsh Gupta wrote:
> Hi Thorsten
>
> On Sat, Mar 6, 2021 at 2:25 AM Thorsten Glaser <tg@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
Reply to: