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