Can use null characters to escape filesystem

[garrynewman]

I haven't totally figured out how this works but one of our users managed to escape the restraints of a subfilesystem to be able to write anywhere on the physical filesystem.

The basic theory is this..

using System.IO;
using Zio;
using Zio.FileSystems;

var physical = new PhysicalFileSystem();

// create a sandboxed subsystem
var subsystem = new Zio.FileSystems.SubFileSystem(physical, "/mnt/c/temp/sandboxed", false);

// cool! this is allowed!
subsystem.WriteAllText("/hello.txt", "hello");

// agh!
UPath path = "/\0\0/mnt/windows/system32/evil.txt";
subsystem.WriteAllText(path, "we escaped!");

This seems to want to write to a T: drive (which I don't have).

But obviously there's some deeper funny business going on here.

I'm not sure whether you even meant for the subfilesystems to work in the sandboxed fashion we're using them, but since this seems like unpredictable/unwanted behaviour I thought I would report it.

[xoofx]

I'm not sure whether you even meant for the subfilesystems to work in the sandboxed fashion we're using them, but since this seems like unpredictable/unwanted behaviour I thought I would report it.

Ouch, sorry for the trouble. It was definitely meant to be sandboxed friendly. Hopefully fixed by commit 13e1ee1. Should be available in 0.16+ soon.

[ninjasploit]

Hey! Saw this issue got created after initial bug report to Garry, I did some further testing after this fix but it seems there are multiple special chars that are able to achieve the same unexpected behavior and escape the sandbox restrictions.

A bit of a weird one though as using Zio directly in a new projects makes the full path end up at T:/ drive (because of padding of the 2 chars inserted to make it bug out, which makes it read the T in /mnt/ as the drive letter for some reason.

Anyways, I'm still able to reproduce this by doing a loop iterating over char codes and testing them to see if I manage to escape the sandboxed system.

(char codes on the left, just replacing the hashtags in the top-most string I use as the test path)

I'd suggest possibly having a check that makes sure the expected base-path of the sandboxed system is part of the full path? That way if the resulting path is not at the expected location it would throw some kind of exception. Might be a bit "better" to do than blocking a lot of different character group categories.

public void EscapeTester(string path)
{        
    // Get current dir for easy testing
    string protectedBasePath = Directory.GetCurrentDirectory().Replace(':', UPath.DirectorySeparator);

    // Initialize the base physical filesystem
    FileSystem fileSystem = new PhysicalFileSystem();
    // Create some sub-system for escape testing blabla...
    SubFileSystem protectedFileSystem = new SubFileSystem(fileSystem, (UPath)$"/mnt/{protectedBasePath}/sandbox");

    // List containing chars resulting in suspicious paths (expecting fullPath to be shorter than the input path since 
    // it should turn "/##/mnt/x/test.txt" into "T:/x/test.txt" OR "X:/test.txt")
    List<int> maliciousChars = new List<int>();
    for (int i = 0; i < 4096; i++)
    {
        try
        {
            string repChar = char.ConvertFromUtf32(i);

            string fullPath = protectedFileSystem.ConvertPathToInternal(path.Replace("#", repChar));
            if (fullPath.Length <= path.Length)
                maliciousChars.Add(i);
            //Console.WriteLine($"{i.ToString("x")} ({char.GetUnicodeCategory(repChar[0])}) - " + fullPath);
        }
        catch (Exception ex)
        {
            //Console.WriteLine(ex);
        }
    }

    // All malicious chars found! Print them
    foreach (int i in maliciousChars)
    {
        try
        {
            string repChar = char.ConvertFromUtf32(i);

            string fullPath = protectedFileSystem.ConvertPathToInternal(path.Replace("#", repChar));
            Console.WriteLine($"{i.ToString("x")} ({char.GetUnicodeCategory(repChar[0])}) - " + fullPath);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex);
        }
    }
}

I was half-asleep when I wrote this, so it might not be the best written example, but it should work!

[xoofx]

Thanks @ninjasploit! Yeah, indeed, we probably need to always check the expanded path to make sure it is within boundaries.

[xoofx]

I have pushed a commit 930974d that should fix this issue. The code was using StarsWith without ordinal comparison, so it could make the actual checks to be completely bypassed.

[ninjasploit]

I have pushed a commit 930974d that should fix this issue. The code was using StarsWith without ordinal comparison, so it could make the actual checks to be completely bypassed.

Superb! 🙌
Going to have a crack at it again to see if I still manage to finesse it somehow.
Will let you know if I do find a way, hopefully I wont though!