6

I have been problem to solve an appointment of Veracode Scanner in my project. I created a function to validate a file but it did not pass in veracode scanner;

Here is the code of my function:

    public static string GetSafeFileName(string fileNameToValidate)
    {
        fileNameToValidate= fileNameToValidate.Replace("'", "''").Replace(@"../", "").Replace(@"..\", "");
        char[] blackListChars = System.IO.Path.GetInvalidPathChars();
        char[] blackListFilename = System.IO.Path.GetInvalidFileNameChars();

        foreach (var invalidChar in blackListChars)
        {
            if (fileNameToValidate.Contains(invalidChar))
            {
                fileNameToValidate = fileNameToValidate.Replace(invalidChar, ' ').Trim();
            }
        }

        string fullPath = Path.GetFullPath(fileNameToValidate);

        string directoryName = Path.GetDirectoryName(fullPath);
        string fileName = Path.GetFileName(fullPath);

        foreach (var invalidChar in blackListFilename)
        {
            if (fileName.Contains(invalidChar))
            {
                fileName = fileName.Replace(invalidChar, ' ').Trim();
            }
        }

        string finalPath = Path.Combine(directoryName, fileName);
        return finalPath;

    }

What are the changes i have to fix the cwe 73 appointment in Veracode scanner? Anybody can help me?

My project is a windows forms running on .net 4.0

Thanks,

Bruno

user3149261
  • 61
  • 1
  • 3

2 Answers2

4

Your problem is that Veracode doesn't actually detect what your code is doing, it detects what cleanser function is (or is not) being called. If you login to Veracode and search for help on "Supported Cleansing Functions" you'll find the list that are detected in your language.

Unfortunately, the list for .Net doesn't include anything for a CWE-73.

So, your solution is to specifically label your function as a cleanser for CWE-73 using a custom cleanser annotation. Search Veracode help for "Annotating Custom Cleansers".

using Veracode.Attributes;

[FilePathCleanser]
public static string GetSafeFileName(string fileNameToValidate)
   {
    ...

That said, your implementation is not secure. Try passing in "C:\Windows\System32\notepad.exe" as a filename to be written to and you'll see the problem.

Blacklisting can only deal with what you expect. Whitelisting is a much stronger approach. Your approach should be based on a whitelist of directories, a whitelist of characters for filenames, and a whitelist of file extensions.

CarryWise
  • 41
  • 5
  • Great post, this pointed me in the correct direction, but I think the attribute is slightly wrong and should be **FilePathCleanser**, as per this posting on the [veracode](https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/LFg2_9xp2XHwMPzQw4YXcw) website – Steve Goodman Dec 20 '18 at 16:24
  • Thanks Steve, the joys of cut & paste. – CarryWise May 29 '19 at 12:55
2

I have tried to solve similar problem but in java context. We used ESAPI as external library. You can review esapi project (for ideas how to realise a better solution in your project):https://github.com/ESAPI/esapi-java-legacy

Actually using esapi validator didn't solve the problem with veracode, but in my opinion reduce the risk for attack. With such a library you can enshure that user can't read file out of parent folder(you must hardcode such a directory) and that the user can't read a file with unproper extension -> you can add such a list with file extensions. But this library cant garantee that you can't manipulate files in the parent directory with allowed extensions.

So if you think that all needed verifications of filepaths are done you must ask for mitigation by design or develope a Map with all needed file resources in the project to enshure that there is no way the user to manipulate external files.

Also if you think that you have created a good filepath verification you can use cleanser annotation to mark your method. Here you can read more about custom cleansers https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/xrEjru~XmUHpO6~0FSae2Q

xyz
  • 812
  • 9
  • 18