0

I ran IBM AppScan tool on a VB.Net source.I am getting one security issue in File.Copy method under Path Traversal category.

Issue Detail - Vulnerability Type - PathTraversal This API accepts a directory, a filename, or both. If user supplied data is used to create the file path, the path can be manipulated to point to directories and files which should not be allowed access or which may contain malicious data or code.

How can i fix this issue?

Imports System.Web.Security.AntiXss
Private Function ProcessFile() As Boolean
    Dim drive As String = String.Empty
    Dim folder As String = String.Empty
    Dim filename As String = String.Empty
    Dim sourcePath As String = String.Empty
    Dim destinationPath As String = String.Empty
    drive = AntiXssEncoder.XmlEncode(String.Format("{0}", System.Configuration.ConfigurationManager.AppSettings("Drive").ToString()))
    folder = AntiXssEncoder.XmlEncode(String.Format("{0}", System.Configuration.ConfigurationManager.AppSettings("Folder").ToString()))
    filename = AntiXssEncoder.XmlEncode(String.Format("{0}", System.Configuration.ConfigurationManager.AppSettings("File").ToString()))

    sourcePath = Path.Combine(drive, folder, filename)
    destinationPath = Path.Combine(drive, folder, "text2.txt")

    Try
        If sourcePath.IndexOfAny(Path.GetInvalidPathChars()) = -1 AndAlso destinationPath.IndexOfAny(Path.GetInvalidPathChars()) = -1 Then
            File.Copy(sourcePath, destinationPath, True)
            Return True
        Else
            Return False
        End If

    Catch ex As Exception
        Return False
    End Try
End Function
Deepak
  • 1
  • 2

2 Answers2

0

It's probably considering AppSettings to be untrusted user input (I've seen AppScan Source do similar with config on a Java project), so it's complaining that you're making a path with untrusted input that could have separators in.

If any of drive, folder and filename did come from untrusted this would definitely be a problem. Assuming however that your config is only accessible to trusted administrators this is nothing. It's pretty stupid that config is treated as an unchecked source, but then taint tracking tools are pretty stupid in general.

The handling of filenames here is rather wacky. It seems very unlikely that XML-encoding filenames before using them is a good idea; the ToString and Format steps are entirely superfluous; and checking the whole path for ‘invalid’ characters doesn't protect against injection from an individual part anyway. Is this stuff an attempt to work around AppScan? The InvalidPathChars check wouldn't help as it doesn't directly encode/validate and return the tainted value, and the XmlEncode would only help if that function were explicitly marked as a validation/encoding function.

It's sad to make code more broken in an attempt to satisfy a blunt instrument of a static analyser. Could you perhaps add a function to be used as a wrapper on AppSettings values and tell AppScan it is a validation/encoding function, so it doesn't think the values are tainted? Or just ignore/silence the bogus warning?

bobince
  • 528,062
  • 107
  • 651
  • 834
0

System.Configuration.ConfigurationManager.AppSettings can be considered as a safe source, you can just exclude the findings so it won't come up again.

On the other hand, this code can be considered as having poor secure coding practice. If you replace "System.Configuration.ConfigurationManager.AppSettings" with something like a web UI input, then the end user has control over the value of "folder" "drive" and "filename", this then becomes a serious path traversal issue.

Judak
  • 11
  • 2