Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Can filesystem::canonical be used to prevent filepath injection for filepaths passed to fstream

I have a public folder pub with subfolders and files in it. A user gives me now a relative filepath, I perform some mappings, and I read the file with fstream and return it to the user.

The problem is now if the user gives me a path like e.g. ../fileXY.txt or some other fancy stuff considering path traversal or other types of filepath injection. fstream is just gonna accept it and read potential files outside of my public pub folder or even worse give them a list of all files on my system etc... .

Before reinventing the wheel, I searched in the filesystem library and I have seen there is this std::filesystem::canonical function and there is quite a talk about the normal form. I have a general question here, can this function and the variant std::filesystem::weakly_canonical be used to prevent this types of vulnerabilities? So basically is it enough?

Further, my system's filesystem library is still in experimental mode and the std::filesystem::weakly_canonical is missing. But I cannot use the canonical because the files must exist in canonical. In my case I have certain mappings and the files dont exist in that sense. So I would need to mimic the weakly_canonical function, but how?

I have seen a related stackoverflow question on realpath for nonexisting paths and he was suggested to repeat the canonical as long as the path exist and then to add the nonexisting part to it, but that is again vulnerable to these type of injections. So do I have to roll my own weakly_canonical or can I somehow mimic it by combining some std::experimental::filesystem functions?

like image 755
ezegoing Avatar asked Apr 10 '19 10:04

ezegoing


2 Answers

Short answer no.

Long answer this is modeled after posix realpath

I understand the source of confusion. From realpath

The realpath() function shall derive, from the pathname pointed to by file_name, an absolute pathname that resolves to the same directory entry, whose resolution does not involve '.', '..

From cppref path you can also see that the double dot is removed. However the path still points to the same file. It's just that redundant elements are removed.

If you are processing values from a db/webapp/whatever where your program has different privileges than the user who supplied the path, you need to sanitize the filename first by escaping double dots. Dots are fine.

Perhaps you can use a regex to escape double dots with a backslash thus rendering them ineffective.

#include <iostream> 
#include <filesystem>
#include <string>
#include <regex>




int main() 
{ 
    
     std::string bad = "../bad/../other";
    std::filesystem::path p(bad);
    
    
    std::cout << std::filesystem::weakly_canonical(p) << std::endl;
    
   
    std::regex r(R"(\.\.)");
    p = std::regex_replace(bad, r, "\\.\\.");
    std::cout << std::filesystem::weakly_canonical(p) << std::endl;
    
}

Output

"/tmp/other"

"/tmp/1554895428.8689194/\.\./bad/\.\./other"

Run sample

like image 145
A. H. Avatar answered Sep 22 '22 19:09

A. H.


I can see how you could employ weakly_canonical() to prevent path traversal - similar to what is described here - by checking that the result is prefixed with your base path. E.g.

#include <iostream>
#include <filesystem>
#include <optional>

// Returns the canonical form of basepath/relpath if the canonical form
// is under basepath, otherwise returns std::nullopt.
// Note that one would probably require that basepath is sanitized, 
// safe for use in this context and absolute.
// Thanks to https://portswigger.net/web-security/file-path-traversal 
// for the basic idea.
std::optional<std::filesystem::path> abspath_no_traversal(
        const std::filesystem::path & basepath,
        const std::filesystem::path & relpath) {

    const auto abspath = std::filesystem::weakly_canonical(basepath / relpath);

    // thanks to https://stackoverflow.com/questions/1878001/how-do-i-check-if-a-c-stdstring-starts-with-a-certain-string-and-convert-a
    const auto index = abspath.string().rfind(basepath.string(), 0);
    if (index != 0) {
        return std::nullopt;
    }
    return abspath;
}

Since I am no security expert, I welcome any corrections.

like image 39
Yuval Avatar answered Sep 21 '22 19:09

Yuval