'How to validate/sanitize an int so Checkmarx notices I validated/sanitized it

I have some code that retrieves numeric data from a source Checkmarx considers untrusted (a file under my control), which at some point I convert to a pair of integers, and range-check both of them with a function like this, before accessing an "unsafe" buffer between both.

int ValidateInt(int value, int min, int max)
{
    if(value < min) { throw new ArgumentOutOfRangeException("value", value, "Too small"); }
    if(value > max) { throw new ArgumentOutOfRangeException("value", value, "Too big"); }
    return value;
}

void AccessBuffer(IntPtr buffer, int bufferLengthInInt32s, int lowerBound, int upperBound)
{
    lowerBound = ValidateInt(lowerBound, 0, bufferLengthInInt32s-1);
    upperBound = ValidateInt(upperBound, 0, bufferLengthInInt32s-1);
    for(int index=lowerBound ; index<upperBound ; index++)
        DoSomething(Marshal.ReadInt32(buffer, index*4));
}

Problem is, it still displeases Checkmarx. In fact, it brought up more "Deserializing untrusted data" after I replaced my more primitive validation (which only checked the lower bound against 0 and the upper bound against the length) by this one!

Is there any way to make Checkmarx understand that we've checked the integers, we know which range they're in, they're no longer "untrusted" now?

c++sharpcheckmarxsecure-coding


Solution 1:[1]

I've been working with CheckMarx for about 6 months now and have had to come up with fixes for about 6 very large enterprise applications. I used Veracode for about 5 years prior.

The big difference is CheckMarx cannot seem to recognize a sanitizer, it often flags my sanitizing functions saying that the data being passed into it has not been sanitized.

All that being said, you cannot do anything malicious with value data types like INT as scripts can't be passed through them. The data type you need to be most worried about, and sanitize wherever appropriate, is String.

Veracode had an VS extension you could download and then tags that you could use to specify it was a sanitizing function. If it saw that tag, it realized that you had made mediation attempts and moved on. I haven't found a way to do this in CheckMarx so I'm constantly having to state its false positive and get it approved by our information security team that reviews things like this.

Solution 2:[2]

The integer validation should have been recognized and sufficed but I don't think Checkmarx recognizes this as a sanitizer. You can change this behavior by overriding the Deserializing Untrusted Data Checkmarx query using Checkmarx Audit.

What Checkmarx recognizes out of the box is the call to ComputeHash methods. I suggest to make a call to any of the available ComputeHash methods from different HashAlgorithm(s) (a strong hashing algorithm is recommended) replacing your current integer checks

SHA256 hashAlgorithm = SHA256.Create();
var lowerBoundHash = hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(lowerBound));
var upperBoundHash = hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(upperBound));
//if (upperBoundHash != expectedUpperBoundHash) {}
//if (lowerBoundHash != expectedLowerBoundHash) {}

Or you can simply point the argument with your security team that this should be marked as not exploitable because of the integer validation in place

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1 James H
Solution 2 securecodeninja

Related Questions