This diff fixes a huge amount of issues that could be triggered by using
the old-fashioned configuration files, i.e menu. They can be activated
by using a line like "/path/to/menu". On most systems, this file will be
parsed with cpp and the result sent through a pipe to WindowMaker.
Anyway, in the code path, memory leaks and buffer overruns await. I have
tried to fix these parts, but in the end it is more or less a rewrite,
whereas I used WINGs whenever possible.
Difference to previous implementation, beside of bugfixes:
- You are free to use single quotes and double quotes in configuration
- Linewrapping is allowed for other pipe'd data, too
- In general, line reading is unified throughout that file
Bugfixes:
- Avoid buffer overrun on lines which have line wrappings, that will happen
ALWAYS, because wtrimspace-usage was erroneously.
- Avoid memory leak, also happened through wtrimspace usage.
- Avoid buffer overrun in separateline if a line ending happens early.
- Actually handle ferror() instead of only feof(), that would result in
endless spinning
- A line wrapping at the end of a file is an error.
In parseMenuCommand, title[300] might get filled with a string of length
300. The string is copied with strcpy, therefore the size would have to be
301 or -- as I propose -- the fixed value 300 gets replaced with
"sizeof(title) - 1". This shows also that the size 300 belongs to title
and it will already be replaced during compile-time into 299.
memset is the last function call in wmalloc, just before it returns the
newly allocated memory. Therefore it is not needed to call it again
after wmalloc call. Although I would prefer to switch wmalloc to a
calloc-based wcalloc function, the compatibility of WINGs for old apps
should be kept.
wtokenfree() does not free the first entry of an array.
If count is 1 then
while (--count)
will be
while (0)
and the inner body of that while-loop will not be entered.
Therefore a memory leak happens every time wtokenfree() is called.
Retrieved-from: http://paldium.homeunix.org/tobias/wmaker/patches/
[crmafra: This patch, altough correct, breaks WPrefs.app, which will be
fixed by the next patch. ]
Problems:
1.
During expansion of path, the resulting path can overflow the supplied
area of PATH_MAX+2 (buffer as well as buffer2). A tampered environment
variable can be used to modify program flow.
Proof:
[note: wmaker has been compiled with propolice]
$ export A="[tested with 4096x A]"
$ GNUSTEP_USER_ROOT="\$A\$A/\$A/\$A/" wmaker --for-real
*** stack smashing detected ***: wmaker terminated
Aborted
2.
Way too many functions handle a return value of NULL for wexpandpath
improperly, resulting in segfaults (and maybe other problems). To
prove the existance of these issues:
Proof:
$ GNUSTEP_USER_ROOT=~nouser wmaker --for-real
wmaker error: could not get password entry for user nouser: Success
Segmentation fault
Solution:
hard exit with error message about what is going on.
3.
The improper parsing of environment variables can lead to expansion
of path names that were not intended to be expanded.
(a) If a string like "$(var" is found, Window Maker tries to expand "var"
(environment variable) although the syntax is wrong.
Proof:
$ export PROOF=foo
$ GNUSTEP_USER_ROOT=/\$\(PROOF wmaker --for-real
wmaker warning: could not find user GNUstep directory (/foo/Defaults/WindowMaker).
(b) If the variable out of a) cannot be resolved, a closing bracket will be
added.
Proof:
$ unset PROOF
$ GNUSTEP_USER_ROOT=/\$\(PROOF wmaker --for-real
./wmaker warning: could not find user GNUstep directory ($(PROOF)/Defaults/WindowMaker).
Author: Tobias Stoeckmann
Retrieved-from: http://paldium.homeunix.org/tobias/wmaker/
Submitted-by: Gilbert Ashley <amigo@ibiblio.org>
Tobias Stoeckmann