Subject: Bug in FvwmCommand

From: "Brennan Cropper" <[email-masked]>
Date: Tue, 24 Aug 1999 11:34:00 +0100

Hi,

I've downloaded and installed 2.3.6, and have found a bug in
FvwmCommand.  FvwmCommand.c assumes that a couple of environment
variables are set, specifically HOME and DISPLAY.  If they are not set,
calls to strlen() and strcpy() with their return values result in
cores.  The questionable calls are located in lines 156 and 165 of
FvwmCommand.c.  The return values of the getenv() call should be checked
before using them.

I discovered the bug by running FvwmCommand as a cron job.  The HOME
environment variable existed in the shell spawned by crond, but the
DISPLAY variable did not, thus causing the core.  Use of FvwmCommand by
my user id worked fine since the environment variables were correctly
set.

Thanks,
Brennan
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Neil Bird <[email-masked]>
Date: Tue, 24 Aug 1999 11:43:17 +0100

> my user id worked fine since the environment variables were correctly

  Oops. Ish.

  Yes, it shouldn't coredump (!), but if you don't specify a FIFO name (-f
or -S I think), and also don't have HOME or DISPLAY set, then how does
FvwmCommand know which FIFO pair to use to communicate with the correct
FvwmCommandS (HOME for location, DISPLAY to ID login - there may be more
than one)?

-- 
=====================- http://www.racaldefence.com/ -===================
 Neil Bird                   |   This is who we are     The time is near
                             |
 mailto:[email-masked] |        There are 130 days remaining
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: "Brennan Cropper" <[email-masked]>
Date: Tue, 24 Aug 1999 11:57:47 +0100

Neil,

To answer your question--I'm not sure.  For a workaround, I just changed
the code to strcpy() in "" as the DISPLAY if the variable didn't exist. 
That fixed *my* problem.  After I saw that that worked, I didn't take
the time to delve into it more than that.  I'm not sure if it's any help
to you, but the pipes setup in my home directory are the following:

".FvwmCommand-foo.bar.nortel.comfoo.bar.nortel.comC|" and
".FvwmCommand-foo.bar.nortel.comfoo.bar.nortel.comM|",

where "foo.bar" is my machine name, of course.  I hope this helps.

Brennan

---

From: Neil Bird <[email-masked]>
Date: Tue, 24 Aug 1999 12:09:19 +0100

> where "foo.bar" is my machine name, of course.  I hope this helps.

  Not really! Not only can I not understand how it actually *worked* given
that DISPLAY would've been set when FvwmCommandS was run (or did you patch
that too?), I can't figure out how the code would have generated those
filenames at all - unless gethostname() returned
"foo.bar.nortel.comfoo.bar.nortel.com", and DISPLAY was "" at FvwmCommandS
startup (if unset, it, too, would dump core).

---

From: "Brennan Cropper" <[email-masked]>
Date: Tue, 24 Aug 1999 12:25:45 +0100

Line 128 of FvwmCommandS.c looks mighty suspect.  I'm not sure of the
inner workings of sprintf(), but sprintf()'ing to the same string as one
of the arguments to the call may be the cause of this.  I inserted two
debug statements just before line 128--one to print "client" and the
other to print "hostname".  "client" = ":0.0", and "hostname" =
"foo.bar.nortel.com".

When this is the case, line 128 (and sprintf()) returns "client" as
being "foo.bar.nortel.comfoo.bar.nortel.com".  As stated before, using
the destination string as one of the arguments to sprintf() is probably
not a smart idea.

Brennan

---

From: Neil Bird <[email-masked]>
Date: Tue, 24 Aug 1999 12:41:33 +0100

> other to print "hostname".  "client" = ":0.0", and "hostname" =

  Hmm. That's not right at /all/, is it?

  I think it ought to be:

  if( name == NULL ) {
    char *dpy_name;

    /* default name */
    home = getenv("HOME");
    if (!home)  home = "";
    f_stem = safemalloc( strlen(home) + strlen(F_NAME) + MAXHOSTNAME + 4);
    strcpy (f_stem, home);
    if (f_stem[strlen(f_stem)-1] != '/') {
      strcat (f_stem, "/");
    }
    strcat (f_stem, F_NAME);

    /* Make it unique */
    strcat (f_stem, "-");
    gethostname(hostname,32);
    dpy_name = getenv("DISPLAY");
    if (!dpy_name)  dpy_name = ":0";
    if (!dpy_name[0]  ||  ':' == dpy_name[0])
      strcat( f_stem, hostname );  /* Put hostname before dpy if not there
*/
    strcat (f_stem, client);

  }else{
    f_stem = name;
  }

  .. in both FvwmCommand.c & FvwmCommandS.c

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 25 Aug 1999 21:56:43 +0200

>   Hmm. That's not right at /all/, is it?
...
>   .. in both FvwmCommand.c & FvwmCommandS.c

Okay, I have applied this patch.

Bye

Dominik ^_^

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]