I've pushed the attached patch removing an integer overflow from telnet.

The overflow occurs went sending 'send dont <value>' but the value
exceeds INT_MAX.

Also, I'm adding inttypes and xstrtoimax from Gnulib. Perhaps it is too
much personal opinion but I find error checking strtol and friends
annoying. I'd prefer using xstrtoimax, since it deals with all the
conditions and platform discrepancies. But feel free to change to a
standard function if you'd like.

Collin

>From a6d9848a32fafa763548e54b44cb094abdac915d Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Fri, 23 Aug 2024 22:48:30 -0700
Subject: [PATCH] telnet: Handle integer overflow gracefully.

* bootstrap.conf (gnulib_modules): Add inttypes and xstrtoimax.
* telnet/commands.c (send_tncmd): Don't allow the integer argument to
'send dont' to overflow.
---
 bootstrap.conf    |  2 ++
 telnet/commands.c | 24 ++++++++++--------------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index e6f56a27..4da15428 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -70,6 +70,7 @@ gitlog-to-changelog
 glob
 intprops
 inttostr
+inttypes
 ioctl
 malloc-gnu
 mempcpy
@@ -118,6 +119,7 @@ xgetcwd
 xgetdomainname
 xgethostname
 xsize
+xstrtoimax
 xvasprintf
 "
 
diff --git a/telnet/commands.c b/telnet/commands.c
index 610e4b6c..0f833860 100644
--- a/telnet/commands.c
+++ b/telnet/commands.c
@@ -92,6 +92,7 @@
 #include "types.h"
 
 #include "xalloc.h"
+#include "xstrtol.h"
 #include "xvasprintf.h"
 #include "xalloc.h"
 
@@ -454,7 +455,7 @@ send_tncmd (void (*func) (int, int), char *cmd, const char *name)
 #if !HAVE_DECL_TELOPTS
   extern char *telopts[];
 #endif
-  int val = 0;
+  intmax_t value = 0;
 
   if (isprefix (name, "help") || isprefix (name, "?"))
     {
@@ -489,30 +490,25 @@ send_tncmd (void (*func) (int, int), char *cmd, const char *name)
   if (cpp)
     {
 #if HAVE_DECL_TELOPTS
-      val = cpp - (char **) telopts;
+      value = cpp - (char **) telopts;
 #else /* !HAVE_DECL_TELOPTS */
-      val = cpp - telopts;
+      value = cpp - telopts;
 #endif
     }
   else
     {
       const char *cp = name;
+      enum strtol_error err = xstrtoimax (cp, NULL, 10, &value, "");
 
-      while (*cp >= '0' && *cp <= '9')
+      if (err == LONGINT_OVERFLOW || value < 0 || value > 255)
 	{
-	  val *= 10;
-	  val += *cp - '0';
-	  cp++;
-	}
-      if (*cp != 0)
-	{
-	  fprintf (stderr, "'%s': unknown argument ('send %s ?' for help).\n",
+	  fprintf (stderr, "'%s': bad value ('send %s ?' for help).\n",
 		   name, cmd);
 	  return 0;
 	}
-      else if (val < 0 || val > 255)
+      else if (err != LONGINT_OK)
 	{
-	  fprintf (stderr, "'%s': bad value ('send %s ?' for help).\n",
+	  fprintf (stderr, "'%s': unknown argument ('send %s ?' for help).\n",
 		   name, cmd);
 	  return 0;
 	}
@@ -522,7 +518,7 @@ send_tncmd (void (*func) (int, int), char *cmd, const char *name)
       printf ("?Need to be connected first.\n");
       return 0;
     }
-  (*func) (val, 1);
+  (*func) (value, 1);
   return 1;
 }
 
-- 
2.46.0

  • telnet: Han... Collin Funk
    • Re: te... Erik Auerswald
      • Re... Simon Josefsson via Bug reports for the GNU Internet utilities
        • ... Erik Auerswald
          • ... Collin Funk
        • ... Collin Funk
        • ... Erik Auerswald
          • ... Collin Funk
          • ... Erik Auerswald
    • Re: te... Erik Auerswald

Reply via email to