Server Directory Traversal - Critical Vulnerabilities

[seceba]

Hello,

On the backend side, I use a code to access my photos.

app.get("/assets/files/*", (req, res) => Directory('files/'));

If I write another link instead of the photo link, I can access whatever file I want to access, which is pretty bad.

When I go to 127.0.0.1:3000/assets/files/notes.txt we should expect to read notes.txt, it works correctly.

But if i go to 127.0.0.1:3000/assets/files/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/..%2f/etc/passwd

I can access the passwd file on my computer and I have not found a way to fix it.

I think this only happens with ..%2f. if I replace %2f with / I cannot access the passwd file.

I think there is a vulnerability with %2f.

[seceba]

I fixed the problem in directory_type_handler.dart by changing

find this line in directory_type_handler.dart:
var match = fileCandidates.firstWhere((file) => file.existsSync());

and add below this code:

  var realpath = await Directory(match.absolute.path.toString()).resolveSymbolicLinks();

          if (!realpath.contains(Directory.current.absolute.path + "/" + directory.path)) {
            print("No!");
            return "No!";
          }

This code prevents it from going to other directories.

[rknell]

Thanks @seceba and @d-markey

I'll get a new version pushed up today. Really good catch

[d-markey]

Trying to write a proper test but I can't get it to work though. I closed my PR until the test code is there but I won't have time before next week. Feel free to look at my branch and possibly fix it if you have time! Thanks.

[d-markey]

BTW I would like to avoid using resolveSymbolicLinks() when checking for the target file path. Having the possibility to symlink a file for download sounds interesting IMHO.

[seceba]

@d-markey Thank you for your solution. I had to use the resolveSymbolicLinks function because I couldn't otherwise find out what expressions like ../../../../ translate the link into.

Using resolveSymbolicLinks seems to be a working solution for now.

But unfortunately this code doesn't allow me to check it because it doesn't provide us with the resolved link.

  final check = File('${directory.path}/${Uri.decodeComponent(virtualPath)}').absolute;
          print(check);

It gives us this:

File: '/Users/mac/Documents/flutter_projects/backend_prod/files/..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..//..///etc/passwd'

But we need to turn this into this: /etc/passwd

Thank you for your effort, and maybe it would be a good idea to hide this problem on github until a permanent and best practice solution is found? permanent solution is found?

@rknell I'm excited about the updates.

[d-markey]

@seceba thanks for your feedback, I've modified my PR to handle all ".." segments and now the test passes.

[rknell]

thanks everyone. New version is now on dart pub.