emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.91k stars 3.32k forks source link

Calls to `PATH_FS.resolve` in `lookupPath` are incorrect if path has symlinks #23024

Open hoodmane opened 6 days ago

hoodmane commented 6 days ago

If link is a symlink to a directory, then link/.. refers to the parent of the target of the link. For instance if we do ln -s . link then realpath link/.. and realpath .. point to the same thing. PATH_FS.resolve will cancel the link with the .. which destroys the information needed to resolve the path correctly.

Here's an example: Call the following a.c:

#include <stdlib.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>

char* join_path(const char* path1, const char* path2) {
    int len1 = strlen(path1);
    int len2 = strlen(path2);
    char* result = malloc(len1 + len2 + 2);
    memcpy(result, path1, len1);
    result[len1] = '/';
    memcpy(result + len1 + 1, path2, len2);
    return result;
}

int main() {
    char template[] = "/tmp/tmpdir.XXXXXX";
    char *tmpdir = mkdtemp(template);
    char* subdir = join_path(tmpdir, "subdir");
    char* current = join_path(subdir, "current");
    char* evil = join_path(current, "../evil");
    char* evil_resolved = join_path(tmpdir, "evil");

    mkdir(subdir, 0777);
    symlink(".", current);
    {
        int src_fd = open(evil, O_CREAT | O_WRONLY, 0777);
        write(src_fd, "abc", 4);
        close(src_fd);
    }
    {
        int target_fd = open(evil_resolved, O_RDONLY);
        printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno));
        char buf[10];
        read(target_fd, buf, 10);
        printf("buf: '%s'\n", buf);
        close(target_fd);
    }
}

Run it:

$ gcc symlinks-suck.c && ./a.out
target_fd: 3, errno: 0 Success
buf: 'abc'
$ emcc a.c && node a.out.js
target_fd: -1, errno: 44 No such file or directory
buf: ''

Note that if we pass -sNODERAWFS then the test works.

hoodmane commented 6 days ago

I think lookupPath should be fixed not to call PATH_FS.resolve and PATH_FS.resolve should be updated to use lookupPath.

hoodmane commented 6 days ago

Calls to PATH.normalize are also probably wrong.

hoodmane commented 6 days ago

I have a fix for this in a local branch but there are too many conflicts right now so I will hold off on opening a PR until some of my other changes land.