Crashes with monitor_resolve_name

Dominik Vogt <[email protected]> Sat, 27 Nov 2021 19:02:29 +0100
Newsgroups gmane.comp.window-managers.fvwm.devel
Message-ID <[email protected]>
There are some NULL pointer crashes and bugs telated to
moitor_resove_ame():

1. The fuction assumes that it's always called with a non-null
   "scr" pointer:

	monitor_resolve_name(const char *scr)
	{
	...
		if (strcmp(scr, "g") =3D=3D 0) {

   This is not the case.  Several places in the source call the
   function without validating that the pointer in non-NULL.

This can be triggered with the command "desk" (no arguments).


2. #1 is because bugs in the parsing code that may be the result
   of a misunderstanding how the parsing functions work:

=2D- snip --
01 void CMD_GotoDesk(F_CMD_ARGS)
02 {
03	...
04	action_cpy =3D strdup(action);
05	action_cpy_start =3D action_cpy;
06	token =3D PeekToken(action_cpy, &action_cpy);
07
08	m =3D monitor_resolve_name(token);
09	if (strcmp(m->si->name, token) !=3D 0)
10		m =3D m_use;
11	else
12		PeekToken(action, &action);
13
14	new_desk =3D GetDeskNumber(m, action, m->virtual_scr.CurrentDesk);
=2D- snip --

Problems here:

 line 02 + 03:  This is pointless.  Neither PeekToken nor the other
          parsing functions modify the "action".

 line 04: fxstrdup() should be used.

 line 06: This seems to assume that PeekToken always returns a
          string pointer.  This is *not* the case!  The parsing
          functions never return an empty string.
          Also, if the caller is not interested in the rest of the
          command line, just pass NULL as second argument:

            token =3D PeekToken(action, NULL);

 line 08: Crashes because of #1.

 line 09: Crashes because token can be NULL.

 line 12: The proper way to skip tokens at the beginning of a
          string is

            action =3D SkipNTokens(action, 1);

 line 14: OK because GetDeskNumber uses MatchToken, which is aware
          of all possible NULL pointers.

=2D-

Also, there's a real parsing bug:

If called with "desk arg1 arg2":

  Case 1:  arg1 is a monitor name.
    -> calls GetDeskNumberGetDeskNumber with action =3D "arg1 arg2"
                                                      ^^^^^^^^^^^
  Case 2:  arg1 is no monitor name.
    -> calls GetDeskNumberGetDeskNumber with action =3D "arg2"
                                                      ^^^^^^

That can't be correct.

=2D-

(Specifying a monitor name is not documented in the man page).

=2D-

Are there other places with unusual monitor name parsing?

Ciao

Dominik ^_^  ^_^

=2D-

Dominik Vogt