none
How to avoid Directory Traversal in my code RRS feed

  • Question

  • How Can I avoid Directory traversal in the following function underlined lines?

     public byte[] GetFile(string filePath) {
                if (filePath == null) throw new ArgumentNullException(nameof(filePath));
                try {
                    switch (StorageProtocol) {
                        case "SB":
                        case "Lcal":
                            using (new NetworkConnection(StoragePath, StorageCredential)) {
    return File.ReadAllBytes($"{StoragePath}/{filePath}");
                            }
                        case "S":
    return File.ReadAllBytes(HttpContext.Current.Server.MapPath($"~/Files/{filePath}"));
                       }


    Regards,

                                        
    • Edited by Gulnar78 Wednesday, October 16, 2019 11:08 AM edit
    Friday, October 4, 2019 7:02 PM

Answers

  • [...] I don't see any traversal from your code. [...] I could not find something wrong in the underline code [...]

    Yes, there is a Path Traversal vulnerability in the underlined code. The problem is that if the method is invoked passing a value for filePath such as "..\..\..\someFolder\someFile", then when the underlined line concatenates it after StoragePath the result is that the caller could be reading a file that is NOT under the expected StoragePath.

    The remedy is to "sanitize" the input. Examine the value that you receive for filePath, and reject it if it contains any characters that you don't allow in your filenames, such as "\" or "..".

    • Marked as answer by Gulnar78 Wednesday, October 16, 2019 11:07 AM
    Monday, October 7, 2019 1:40 PM
    Moderator

All replies

  • What exactly do you mean???

    Please remember to mark the replies as answers if they help and unmarked them if they provide no help, this will help others who are looking for solutions to the same or similar problem. Contact via my Twitter (Karen Payne) or Facebook (Karen Payne) via my MSDN profile but will not answer coding question on either.

    NuGet BaseConnectionLibrary for database connections.

    StackOverFlow
    profile for Karen Payne on Stack Exchange

    Friday, October 4, 2019 8:13 PM
    Moderator
  • Hi Gulnar,

    Thank you for posting here.

    According to your description, you want to avoid Directory Traversal. However, I don't see any traversal from your code.

    Also, I could not find something wrong in the underline code. I hope that you could describe it more clearly.

    Best Regards,

    Jack


    MSDN Community Support
    Please remember to click "Mark as Answer" the responses that resolved your issue, and to click "Unmark as Answer" if not. This can be beneficial to other community members reading this thread. If you have any compliments or complaints to MSDN Support, feel free to contact MSDNFSF@microsoft.com.

    Monday, October 7, 2019 2:46 AM
    Moderator
  • [...] I don't see any traversal from your code. [...] I could not find something wrong in the underline code [...]

    Yes, there is a Path Traversal vulnerability in the underlined code. The problem is that if the method is invoked passing a value for filePath such as "..\..\..\someFolder\someFile", then when the underlined line concatenates it after StoragePath the result is that the caller could be reading a file that is NOT under the expected StoragePath.

    The remedy is to "sanitize" the input. Examine the value that you receive for filePath, and reject it if it contains any characters that you don't allow in your filenames, such as "\" or "..".

    • Marked as answer by Gulnar78 Wednesday, October 16, 2019 11:07 AM
    Monday, October 7, 2019 1:40 PM
    Moderator
  • I'd have to ask the question of whether StoragePath is some user-provided value or a configuration value. If it is a configuration value then why would you care about what directory it is pointing to other than somebody somewhere said "it could be an issue". Seems there are a lot of threads appearing this past week related to "security scans" detecting "incorrect code" that are bogus.

    If StoragePath is provided by a user and you want to ensure it doesn't go outside some fixed path structure you have then you have a couple of options. The first option is to assume the path is rooted to your "root" directory and resolve it relative to that. The second option is to use the Path.GetFullPath method to force the path to an absolute path (which handles relative paths) and then call Path.GetRelativePath to get the path relative to your root path. If it can't then the original path was not under your "root". Alternatively Win32 has the PathCommonPrefix function which determines the shared root of two paths. If the common path isn't in your root path structure then neither was the input file.

    I'd also point out that you shouldn't concat paths using /. Path.Combine is provided to help with this although it doesn't behave correctly in all cases. Nevertheless you need to concat the paths taking start/end slashes in mind.


    Michael Taylor http://www.michaeltaylorp3.net

    Monday, October 7, 2019 2:00 PM
    Moderator