fix for stat(2) path traversal using fstat

[jaromil]

Hi there!

Here is my fix to retrieve correct file information on POSX systems when along the path traversal a directory has no executable flag.

From stat(2) manpage:

"No permissions are required on the file itself, but—in the case of stat(), fstatat(), and lstat() execute (search) permission is required on all of the directories in pathname that lead to the file."

At the moment this is an incomplete fix: it breaks usage of lstat(2) on POSIX systems, due to change of the function signature passed to _file_info_().

Kindly provide feedback on how to proceed. IMHO the usage of a function pointer passed as argument of file_info is a design burden: a simple flag can be used.

ciao

p.s. for the impatient, a quick non-breaking fix is applied in the lfs version shipped in my software.

[moteus]

I think you should also close file inside if statement

[hishamhm]

@jaromil Feel free to simplify the internals if it aids with the bugfix and preserves portability!

[jaromil]

Thanks for your feedbacks, knowing your interest now I'll rework the PR into something more presentable and passing tests.

[FractalU]

@jaromil Your fix doesn't solve anything. In fact, it makes the code a lot worse in maintainability, readability and in functionality.

Let me explain. From reading your proposal and reviewing your changes I see that you try to convince us to use fstat instead of stat, because stat has a limitation that fstat doesn't have.

"No permissions are required on the file itself, but—in the case of stat(), fstatat(), and lstat() execute (search) permission is required on all of the directories in pathname that lead to the file.

The three functions stat, fstatat and lstat have execute permission restriction on the directories whose path is a prefix of the path name parameter of these functions. Those restriction only apply to functions with the path name parameter. In contrast to these three functions, fstat doesn't have a path parameter and therefore doesn't have these restrictions. Now you might think that using fstat avoids these restrictions and is therefore a better choice, but in reality, the open function which is being used to acquire the file descriptor fd has the exact same limitations with the permissions.

To illustrate the limitation I have the following example. Suppose I want to get the file information from a file with the path /a/b/c/d/text.txt. If I use stat then the directories./, /a, /a/b, /a/b/c, /a/b/c/d must all have the execute (search or traverse) permission in order for text.txt to be accessible. Otherwise, this file isn't accessible. That's the limitation. Now suppose to use fstat instead. In order to get the file information from /a/b/c/d/text.txt via fstat I need to open it first via the open function. That open function returns a file descriptor which I then supply it to fstat. Of course, that file descriptor has to be closed via close. While fstat doesn't require the /, /a, /a/b, /a/b/c, /a/b/c/d directories to have execute (search or traverse) permission, the open function does. Therefore, the execute permission is required for directories leading to text.txt in order to acquire the file information, no matter whether stat of fstat is being used.

Your changes in this PR can be simplified to the following code.

#define STAT_FUNC real_stat

...

#ifndef _WIN32

static int real_stat(const char *pathname, STAT_STRUCT *buf) {
	int fd = open(pathname, O_RDONLY);
	if(fd<0) {
		return -1;
	}
	int err1 = fstat(fd, buf);
	int err2 = close(fd);
	return !err1 && !err2 ? 0 : -1;
}

#endif

With the simplification, I created a new function called real_stat. real_stat internally uses fstat in order to avoid the execute permission limitation for directories leading to pathname. The problem with it is that open has this limitation. Therefore real_path has the exact same execute permission requirement for directories leading to pathname as stat. No matter how much this code gets turned and twisted, real_path gets still the same limitation. The execute permission requirement for directories leading to pathname cannot be avoided in the unix systems and actually makes perfect sense. These directories need execute (search or traverse) permission in order for the file in pathname to be reachable. In a certain way, real_stat is more limiting that stat. real_stat needs read permission for the file because of open(pathname, O_RDONLY); whereas stat doesn't need any permission on the file.

In summary. Looking from a logical perspective, using fstat instead of stat doesn't relax any restriction. In contrary, it add restrictions, because using fstat requires the open function which then requires some permission such as read. Also, the additional code adds maintenance burden.