shm_open() and shm_unlink() accept relative paths. As the test case below demonstrates, if you give a relative path to shm_open, you will be able to open files anywhere in the system (assuming that you have the right permissions). Test case: #include <fcntl.h> int main (void) { shm_open ("../../tmp/abc", O_CREAT | O_RDWR, 0666); perror ("shm_open"); } Expected output: shm_open: Invalid argument Actual output: shm_open: Success
I believe the fix for this bug should be related to the fix for #14752: shm_open and shm_unlink should validate the name before doing anything else, and the validation should require that, after stripping initial slashes, the string is no longer than NAME_MAX and contains no slashes.
There should be a third constraint: no directories. Consider: shm_open (".", O_RDONLY, 0666); It contains no slashes, it's lower than NAME_MAX but is still an invalid name. errno should be EINVAL in this case. Also consider this code: mkdir ("/dev/shm/some-name", 0666); shm_open ("some-name", O_RDONLY, 0666); Currently it works. If you specify O_WRONLY and O_RDWR you get EINVAL from open(2). In my opinion, errno should be EISDIR in any case (O_RDONLY, O_WRONLY and O_RDWR).
EISDIR does not really make sense because, from the API standpoint of POSIX SHM, "directories" do not exist in this namespace. Of course a malicious program can create directories under /dev/shm, though, leading to an error condition that programs must be able to deal with. I'm not sure what the best error code would be. How would you propose checking for directories? The obvious robust approach is fstat, but that does increase the cost of each shm_open operation moderately. If you just want to reject invalid names, "." and ".." would be the only two that need consideration. But if you want to protect applications from maliciously-created directories, the fstat approach might be needed. By the way, I'm fairly sure that such directories are not relevant to programs that use shm_open in a secure manner. Such programs must have the first user open the file with O_EXCL|O_CREAT, and notify other users out-of-band of the filename for the shared memory object. Otherwise there is no way of knowing (within the SHM API) whether the object you opened belongs to another malicious user; the possibility that it's a directory is a much lesser danger than the possibility that it's an actual shared memory object owned by another user. So I rather question whether protection against directories matters.
I agree with you about EISDIR being a bad error number. I raised the problem of directories because I do have a directory in my /dev/shm. It's a directory created by Byobu (http://byobu.co/) used as cache. Byobu is written in Bash, so it's not affected by the behavior of shm_open(). However it shows that the possibility of encountering directories in /dev/shm is not remote and also shows that such directories aren't created maliciously (although we might say Byobu is buggy). Whether it makes sense to create directories in /dev/shm or not, I still think that shm_open() with O_RDONLY should not open them: otherwise every call to read() will fail with EISDIR (which, as you noted, is not a good error number). If we allow shm_open() to return directories, then we are returning broken file descriptors. About fstat: you might use it just for O_RDONLY. I think this is a feasible approach as people expect some delay when accessing resources. Also, fstat should not be noticeably slow.
Fixed by 5d30d853295a5fe04cad22fdf649c5e0da6ded8c.
Let's treat this as hardening, and not a security issue as such.