Fri 01 Apr 2016 11:01:50 PM UTC, comment #5:
Hello,
If I open more than one file in nano 2.5.3 on MacOS X, it gets into a weird
state in the file input management code only when opening >1 file using
absolute pathnames. Here is what happened:
1) On accessing the second file, troubl begins in has_valid_path, when we are
executing the compound expression "dirname(mallocstrcpy(NULL, filename));".
2) This compound operation sets a pointer "dest" to 0x100400100 in my case and
fills it with some data.
3) It calls dirname and assigns the result to parentdir. But this is not safe
according to the dirname manpage on MacOS X:
"The dirname() function returns a pointer to internal storage space allocated
on the first call that will be overwritten by subsequent calls. Other vendor
implementations of dirname() may modify the contents of the string passed to
dirname(); if portability is desired, this should be taken into account when
writing code which calls this function."
4) Due to the issue in item (3) two kinds of undefined behavior can happen:
a) if dirname() is implemented where the memory is internal storage, the later
free(parentdir) destroys someone else's storage without permission
b) if dirname() is implemented where the memory is modifying the source
string, then we could be allocating a region at a specific start pointer and
freeing from a different start pointer inside the region
5) On this OS, I believe we are hitting case (a):
mallocstrcpy(NULL, filename) returns 0x100400100 in an example case.
parentdir contains 0x101001200 which is pretty far away (internal storage most
likely).
6) At the end of the function free(parentdir) is called illegally.
7) The illegal free() crashes the program:
/* Verify that the containing directory of the given filename exists. */
bool has_valid_path(const char *filename)
{
char *parentdir;
struct stat parentinfo;
bool validity = FALSE;
if (strrchr(filename, '/') == NULL)
parentdir = mallocstrcpy(NULL, ".");
else
parentdir = dirname(mallocstrcpy(NULL, filename)); *** UNDEFINED BEHAVIOR HERE ***
if (stat(parentdir, &parentinfo) == -1) {
if (errno == ENOENT)
statusbar(_("Directory '%s' does not exist"), parentdir);
else
statusbar(_("Path '%s': %s"), parentdir, strerror(errno));
} else if (!S_ISDIR(parentinfo.st_mode)) {
statusbar(_("Path '%s' is not a directory"), parentdir);
} else {
if (access(parentdir, X_OK) == -1)
statusbar(_("Path '%s' is not accessible"), parentdir);
else
validity = TRUE;
}
free(parentdir); *** ILLEGAL FREE CRASHES HERE ***
if (!validity)
beep();
return validity;
}
Thanks,
Matthew Hall
|