Re: [PATCH 3/3] telnet: Avoid command evaluation crashes.

[Simon Josefsson]

Erik Auerswald <auerswal@unix-ag.uni-kl.de> writes:

>> You might want to take a look at:
>>    
>> <https://git.hadrons.org/cgit/debian/pkgs/inetutils.git/tree/debian/patches/0004-telnet-Add-checks-for-option-reply-parsing-limits.patch>
>
> Thanks for pointing out that patch.  Without it telnet crashes when
> it starts the log in process:
...
> @Simon: if you think it is OK to add this patch to GNU Inetutils,
>         feel free to just go ahead and do so.

I can reproduce the problem, and committed the patch.

> Then there is the nagging issue that I did not see how these
> changes prevent the 5000 A bytes from overflowing the now
> 512 byte buffer.  Could it be that there are other bounds
> checks that should be adjusted as well to account for this
> overhead of up to five bytes?  In addition to, not as a
> replacement of, the checks from the patch.

Valgrind doesn't complain on the patched version, but does on the
unpatched version:

==1818584== Invalid write of size 1
==1818584==    at 0x1146AB: env_opt_add (telnet.c:1776)
==1818584==    by 0x11470F: env_opt_add (telnet.c:1731)
==1818584==    by 0x11498E: env_opt.part.0 (telnet.c:1617)
==1818584==    by 0x115C15: telrcv (telnet.c:2144)
==1818584==    by 0x116054: Scheduler (telnet.c:2437)
==1818584==    by 0x1165C6: telnet (telnet.c:2497)
==1818584==    by 0x11087D: tn (commands.c:2869)
==1818584==    by 0x10D85B: main (main.c:407)
==1818584==  Address 0x4a813a0 is 0 bytes after a block of size 512 alloc'd
==1818584==    at 0x483AD7B: realloc (vg_replace_malloc.c:834)
==1818584==    by 0x11478A: env_opt_add (telnet.c:1741)
==1818584==    by 0x11470F: env_opt_add (telnet.c:1731)
==1818584==    by 0x11498E: env_opt.part.0 (telnet.c:1617)
==1818584==    by 0x115C15: telrcv (telnet.c:2144)
==1818584==    by 0x116054: Scheduler (telnet.c:2437)
==1818584==    by 0x1165C6: telnet (telnet.c:2497)
==1818584==    by 0x11087D: tn (commands.c:2869)
==1818584==    by 0x10D85B: main (main.c:407)

This seems sufficient reason to just apply it.

/Simon

signature.asc
Description: PGP signature