Re: TFTP client crash seems to be caused by missing bounds check in makeargv()

2022-09-08 Thread Simon Josefsson via Bug reports for the GNU Internet utilities 
Erik Auerswald  writes:

> Hi Simon,
>
> On Tue, Sep 06, 2022 at 08:05:04PM +0200, Simon Josefsson wrote:
>> Erik Auerswald  writes:
>> > On 04.09.22 17:34, Erik Auerswald wrote:
>> >> On 03.09.22 19:07, Erik Auerswald wrote:
>> >>> On Sat, Sep 03, 2022 at 05:39:45PM +0200, Simon Josefsson wrote:
>>  [...]
>>  did you notice some fuzzing report that wasn't fixed?
>> >>> [...]
>> >>> * Problems found in tftp (the code did not change since the report):
>> >>>
>> >>>    * Untrusted Pointer Dereference in getcmd() at
>> >>> inetutils/src/tftp.c:878
>> >>>  
>> >>> https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00018.html
>> >> That seems to be a missing bounds check in makeargv(), similar
>> >> to the old, now fixed, code in telnet.
>> >> I'll look into creating a nice reproducer instead of the one
>> >> found by the fuzzer, adding a test case, and fixing the bug.
>> >
>> > That is harder than expected….  Is there a reason *not* to use
>> > the crash input found by the fuzzer in a test for GNU Inetutils?
>> 
>> More testing would be great!
>
> I expect to find the time to finalize this during the coming weekend.
> I intend to use perl to write the fuzzer-generated test input provided
> by AiDai into the tftp client, similar to the telnet tests you have
> added for the respective crash bugs.
>
> After adding the test case I intend to commit the attached patch for tftp.
>
> What do you think?

Looks good to me, please add it.

Better would be to fix arbitrary limitations like this, but I'm mixed
about introducing yet further changes (and new bugs..) compared to BSD
tools.  For now I prefer fixing problems in the most minimal way and
similar way that has been fixed in other implementations.

/Simon

> Thanks,
> Erik
>
> From 0cb957adf678cb32936e5e9ad5727c8ad5e28825 Mon Sep 17 00:00:00 2001
> From: Erik Auerswald 
> Date: Sun, 4 Sep 2022 17:36:22 +0200
> Subject: [PATCH] tftp: ignore excess arguments
>
> When given too many arguments to a command at the tftp cli,
> the buffer used to hold the arguments would overflow.  This
> could result in a crash.
>
> The problem was reported by AiDai in
> .
>
> * src/tftp.c (makeargv): Do not overflow argument buffer.
> ---
>  NEWS   |  6 ++
>  src/tftp.c | 10 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index 07115db1..6edeabea 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -8,6 +8,12 @@ GNU inetutils NEWS -- history of user-visible changes.
>  are 0xff 0xf7 (IAC EC) or 0xff 0xf8 (IAC EL).  Reported in:
>  
> .
>  
> +** tftp
> +
> +*** Avoid crashing when given unexpected or invalid commands from tty.
> +Reported by AiDai in
> +.
> +
>  * Noteworthy changes in release 2.3 (2022-07-08) [stable]
>  
>  ** telnet
> diff --git a/src/tftp.c b/src/tftp.c
> index 42abbb4a..6b1e209e 100644
> --- a/src/tftp.c
> +++ b/src/tftp.c
> @@ -122,7 +122,10 @@ static int fromatty;
>  char mode[32];
>  char line[200];
>  int margc;
> -char *margv[20];
> +
> +#define TFTP_MAX_ARGS 20
> +
> +char *margv[TFTP_MAX_ARGS];
>  char *prompt = "tftp";
>  jmp_buf toplevel;
>  void intr (int signo);
> @@ -914,6 +917,11 @@ makeargv (void)
>   cp++;
>if (*cp == '\0')
>   break;
> +  if (margc + 1 >= TFTP_MAX_ARGS)
> + {
> +   fprintf (stderr, "Ignoring excess arguments.\n");
> +   break;
> + }
>*argp++ = cp;
>margc += 1;
>while (*cp != '\0' && !isspace (*cp))


signature.asc
Description: PGP signature

Re: [BUG][PATCH] Someone described a remote DoS Vulnerability in telnetd (dereference NULL pointer ---> SEGV)

2022-09-08 Thread Simon Josefsson via Bug reports for the GNU Internet utilities 
Guillem Jover  writes:

> [ Resending with To trimmed. ]
>
> Hi!
>
> On Tue, 2022-08-30 at 22:57:51 +0200, Guillem Jover wrote:
>> On Sun, 2022-08-28 at 14:40:44 +0200, Erik Auerswald wrote:
>> > On Sat, Aug 27, 2022 at 07:37:15PM +0200, Erik Auerswald wrote:
>> > > someone has described a remote DoS vulnerability in
>> > > many telnetd implementations that I just happened to
>> > > stumble over:
>> > > 
>> > > https://pierrekim.github.io/blog/2022-08-24-2-byte-dos-freebsd-netbsd-telnetd-netkit-telnetd-inetutils-telnetd-kerberos-telnetd.html
>> > > 
>> > > The vulnerability is a NULL pointer dereference when
>> > > reading either of two two byte sequences:
>> > > 
>> > > 1: 0xff 0xf7
>> > > 2: 0xff 0xf8
>> > > 
>> > > The blog shows GNU Inetutils' telnetd as vulnerable:
>> > > 
>> > > https://pierrekim.github.io/blog/2022-08-24-2-byte-dos-freebsd-netbsd-telnetd-netkit-telnetd-inetutils-telnetd-kerberos-telnetd.html#remote-dos-inetutils
>> 
>> This has been assigned CVE-2022-39028 (I think from the Debian pool),
>> after I reported it to the Debian security team.
>
> While it might have been nice to get this in the commit message, I
> think it would still be nice to add a reference in the NEWS. :)

Added, thank you.

https://git.savannah.gnu.org/cgit/inetutils.git/commit/?id=6c3c6acaf352151c6155a8cd78fe490148d0e22a

/Simon


signature.asc
Description: PGP signature