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") == 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:

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

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

--

Also, there's a real parsing bug:

If called with "desk arg1 arg2":

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

That can't be correct.

--

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

--

Are there other places with unusual monitor name parsing?

Ciao

Dominik ^_^  ^_^

--

Dominik Vogt

Reply via email to