Bug 51406 - redundant and buggy code
redundant and buggy code
Status: CLOSED NOTABUG
Product: Red Hat Linux
Classification: Retired
Component: newt (Show other bugs)
9
All Linux
medium Severity medium
: ---
: ---
Assigned To: Eido Inoue
Aaron Brown
: Security
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2001-08-10 05:28 EDT by Need Real Name
Modified: 2007-04-18 12:35 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2003-12-12 12:53:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Need Real Name 2001-08-10 05:28:56 EDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.2) Gecko/20010701

Description of problem:
You have a buggy code in all version of newt

This patch is prodused by OpenWall and ALT Linux Team:
(see aditional information below)

How reproducible:
Always

Steps to Reproduce:
Just apply this patch
	

Actual Results:  Security will be more higher on this program

Additional info:

diff -ur newt-0.50.18.orig/form.c newt-0.50.18/form.c
--- newt-0.50.18.orig/form.c    Fri Apr 28 19:35:55 2000
+++ newt-0.50.18/form.c Fri May 18 08:55:00 2001
@@ -147,12 +147,13 @@
 
 static int Gpm_Open(Gpm_Connect *conn, int flag)
 {
-  char tty[32];
   char *term;
   int i;
   struct sockaddr_un addr;
   Gpm_Stst *new;
+#ifndef SO_PEERCRED
   char* sock_name = 0;
+#endif
 
   /*....................................... First of all, check xterm */
 
@@ -194,21 +195,16 @@
       if (flag>0)
         {  /* forced vc number */
           conn->vc=flag;
-          sprintf(tty,"/dev/tty%i",flag);
         }
       else if (flag==0)  /* use your current vc */
         {
           char *t = ttyname(0); /* stdin */
           if (!t) t = ttyname(1); /* stdout */
           if (!t) goto err;
-          strcpy(tty,t);
-          if (strncmp(tty,"/dev/tty",8) || !isdigit(tty[8]))
+          if (strncmp(t,"/dev/tty",8) || !isdigit(t[8]))
             goto err;
-          conn->vc=atoi(tty+8);
+          conn->vc=atoi(t+8);
         }
-      else /* a default handler -- use console */
-        sprintf(tty,"/dev/tty0");
-
     }
 
   new->info=*conn;
@@ -223,6 +219,7 @@
           goto err;
         }
 
+#ifndef SO_PEERCRED
       bzero((char *)&addr,sizeof(addr));
       addr.sun_family=AF_UNIX;
       if (!(sock_name = tempnam (0, "gpm"))) {
@@ -233,6 +230,7 @@
                 sizeof (addr.sun_family) + strlen (addr.sun_path))==-1) {
         goto err;
       } /*if*/
+#endif
 
       bzero((char *)&addr,sizeof(addr));
       addr.sun_family=AF_UNIX;
@@ -299,11 +297,13 @@
     }
   while(gpm_stack);
   if (gpm_fd>=0) close(gpm_fd);
+#ifndef SO_PEERCRED
   if (sock_name) {
     unlink(sock_name);
     free(sock_name);
     sock_name = 0;
   } /*if*/
+#endif
   gpm_flag=0;
   return -1;
 }

---------------------------
You can find it at ALT Linux
ftp.altlinux.ru/pub/distributions/ALTLinux/Sisyphus/SRPMS/newt-*
and in Owl (OpenWall Linux) http://www.openwall.com/Owl/
Comment 1 Elliot Lee 2001-08-26 15:52:21 EDT
Please provide a more detailed description of the problem than "buggy code"
before requesting a fix to the problem. :)
Comment 2 Elliot Lee 2001-08-26 15:52:55 EDT
Please provide a more detailed description of the problem than "buggy code"
before requesting a fix to the problem. :)
Comment 3 Need Real Name 2001-08-28 01:33:48 EDT
Well. I give to you a more detailed description:
1. You don't need to use a tty[32] variable.(see first part of the patch)
2. It's not secure to leave sockets it tmp dir(see in patch between SO_PERCRED).
You can safely remove it (you already have an open file descriptor) 'cause you
need it only to open connection with gpm.

--------------------
Comment 4 Kjartan Maraas 2003-03-31 15:37:53 EST
Any news on this? Has it been fixed?
Comment 5 Mark J. Cox (Product Security) 2003-12-12 04:04:47 EST
Changing version to 9 as this still applies.
Comment 6 Eido Inoue 2003-12-12 12:53:19 EST
I don't see this as relevant. While the author of the patch makes an
effort to remove the fixed-size string, the fixed size string is only
used to hold the results returned by ttyname(), which is a trusted
POSIX function whose return value is well known.

as for the second part of the patch, you do not #define SO_PEERCRED
anywhere, so I fail to see how the addition of #ifndef SO_PEERCRED has
any effect.

Note You need to log in before you can comment on or make changes to this bug.