Jak nie udostępniać plików do ściągnięcia

3

Ostatnio dodawałem prostą funkcję do pewnej aplikacji webowej: umożliwienie ściągnięcia pliku z dysku. Były to pliki Excela i znajdowały się w katalogu ~/App_Data/reports.

Siłą rzeczy takie pliki nie mają ID. Mają nazwę. I po nazwie właśnie je się ściągało.

Kod otwierający plik do ściągnięcia można by napisać tak:

public Stream OpenFile(string fileName)
{
    string reports_dir = HttpContext.Current.Server.MapPath("~/App_Data/reports");
    string full_path = Path.Combine(reports_dir, fileName);

    return File.OpenRead(full_path);
}

Użytkownik klika na jakiś link http://app.com/download?fileName=report1.xlsx i dostaje ładne okienko do ściągnięcia.

Piknie? Piknie!

Ale! Wejdźcie teraz na link

http://app.com/download?fileName=../../web.config . I co? Już nie tak piknie, prawda?

Takie zagrożenie siedzi u mnie w głowie od roku 2008, kiedy to na ówczesnym silniku mojego bloga (BlogEngine.NET) wykryto security issue zafixowany w tym commicie: http://blogengine.codeplex.com/SourceControl/changeset/41ac0cea1129. Nieźle, co? Setki blogów stojących na BE.NET miały hasła administratora plaintextem wpisane w web.config (bo nie było innej możliwości). A jakiś cwaniak zauważył, że gdzieś w silniku, w środku, siedzi sobie JavaScriptHandler.cs serwujący pliki JS na podstawie nazwy. Tyle że nie sprawdza czy zwraca faktycznie JSy i zwracał cokolwiek się chciało.

Dość prosto można ominąć taką lukę albo sprawdzając rozszerzenia żądanych plików (jak to zrobiono w BE.NET) albo bardziej… “pro”, moim zdaniem. O tak:

public Stream OpenFile(string fileName)
{
    string reports_dir = HttpContext.Current.Server.MapPath("~/App_Data/reports");

    var reports_dir_info = new DirectoryInfo(reports_dir);
    var fileInfo = reports_dir_info.EnumerateFiles(fileName).Single();
    return fileInfo.OpenRead();
}

DirectoryInfo nie pozwoli wyjść “w górę” i udostępnia tylko pliki które faktycznie w danym katalogu się znajdują.

Share.

About Author

Programista, trener, prelegent, pasjonat, blogger. Autor podcasta programistycznego: DevTalk.pl. Jeden z liderów Białostockiej Grupy .NET i współorganizator konferencji Programistok. Od 2008 Microsoft MVP w kategorii .NET.Więcej informacji znajdziesz na stronie O autorze.Napisz do mnie ze strony Kontakt. Dodatkowo: Twitter, Facebook, YouTube.

3 Comments

  1. Pingback: dotnetomaniak.pl

  2. warto zwrócić uwagę na jeszcze jedną sprawę, jeśli pliki nazywają się 1.txt, 2.txt etc to można sobie ręcznie pobrać całą zawartość katalogu – nawet jeśli nie powinniśmy oglądać takich plików. Więc, budując takie coś może lepiej od razu ze 3 guidy do kupy zebrać i to użyć jako nazwy pliku – entropia nas uratuje ;)

  3. Problem, ktory opisales jest znany jako “Directory Traversal Vulnerability”
    Troche mozna poczytac np. tutaj http://www.acunetix.com/websitesecurity/directory-traversal/

    Jest to metoda ataku znana od dawna. Wg. mnie framework webowy powiniec miec wbudowana ochrone przed tedo typu atakami.

    Jezeli robimy samemu takie zabezpieczenie to uwazam ze trzeba zwrocic uwage na nast. rzeczy:
    1. Nie robic “security by obscurity” :)
    2. W parsowanej sciezce zwrocic uwage na ciagi wystepujace w srodku:”..” i na poczatku:”//”,”/”
    3. W katalogu (w Twoim przypadku) “reports” trzymac tylko raporty, zadnych innych plikow.

    Zagadnienie jest na pewno o wiele szersze i zasluguje na wiekszy wywod ;)