Re: NULL Pointer Dereference in help()

[Erik Auerswald]

Hi,

On Thu, Dec 23, 2021 at 09:15:59PM +0800, AiDai wrote:
> Line 1: # NULL Pointer Dereference in help() at
> inetutils/telnet/commands.c:3094

Thanks for fuzzing GNU inetutils!

> AiDai <wyxai...@gmail.com> 于2021年12月23日周四 21:13写道：
> 
> > **command:**
> >
> > ```
> > ./telnet < ./poc
> > ```
> >
> >> ## Description
> >>
> >> A NULL Pointer Dereference was discovered in help() at
> >> inetutils/telnet/commands.c:3094. The vulnerability causes a segmentation
> >> fault and application crash.

This problem occurs when asking for the help text for the help command
using the name "help" instead of "?":

    $ telnet/telnet
    telnet> help ?
    print help information
    telnet> help help
    Segmentation fault (core dumped)

The cause is no help text for "help" in cmdtab2.  This is valid according
to the comments describing "struct Command":

    typedef struct
    {
      const char *name;             /* command name */
      const char *help;             /* help string (NULL for no help) */
      int (*handler) ();            /* routine which executes command */
      int needconnect;              /* Do we need to be connected to execute? */
    } Command;

But the code in help() only checks this when given no arguments, not
with arguments to "help".

The first attached patch "inetutils-telnet-dont_print_nonexistent_help.patch"
fixes this.

This is the correct fix for this issue.

The second attached patch "inetutils-telnet-use_help_helptext.patch" adds
the existing help text for the help command to cmdtab2.  This also fixes
the NULL pointer dereference, but only for the "help" entry.  This suffices
currently, because this is the only entry without a help text, but
providing no help text is intended as valid and thus must be handled.  Thus
this is intended to improve the user experience, not as a fix for following
a NULL pointer into chaos and madness.

Thanks,
Erik
-- 
Golden rule #12: When the comments do not match the code, they probably
                 are both wrong.
                        -- Steven Rostedt

diff --git a/telnet/commands.c b/telnet/commands.c
index 9e04944f..3b05800f 100644
--- a/telnet/commands.c
+++ b/telnet/commands.c
@@ -3090,7 +3090,7 @@ help (int argc, char *argv[])
 	printf ("?Ambiguous help command %s\n", arg);
       else if (c == (Command *) 0)
 	printf ("?Invalid help command %s\n", arg);
-      else
+      else if (c->help)
 	printf ("%s\n", c->help);
     }
   return 0;

diff --git a/telnet/commands.c b/telnet/commands.c
index 9e04944f..4cd01beb 100644
--- a/telnet/commands.c
+++ b/telnet/commands.c
@@ -2931,7 +2931,7 @@ static char crmodhelp[] = "deprecated command -- use 'toggle crmod' instead";
 static char escapehelp[] = "deprecated command -- use 'set escape' instead";
 
 static Command cmdtab2[] = {
-  {"help", 0, help, 0},
+  {"help", helphelp, help, 0},
   {"escape", escapehelp, setescape, 0},
   {"crmod", crmodhelp, togcrmod, 0},
   {NULL, NULL, NULL, 0}