CodeQL documentation
CodeQL resources

Potentially dangerous use of non-short-circuit logic

ID: cs/non-short-circuit
Kind: problem
Security severity: 
Severity: error
Precision: high
Tags:
   - reliability
   - correctness
   - logic
   - external/cwe/cwe-480
   - external/cwe/cwe-691
Query suites:
   - csharp-security-and-quality.qls

Click to see the query in the CodeQL repository

The | and & logical operators, known as non-short circuit operators, should not be used. Using a non-short circuit operator reduces the efficiency of the program, is potentially confusing and can even lead to the program crashing if the first operand acts as a safety check for the second.

Recommendation

If the non-short circuit operator is unintended then replace the operator with the short circuit equivalent. Sometime a non-short circuit operator is required because the operands have side effects. In this case it is more efficient to evaluate both operands separately and then use a short circuit operator to combine the results.

Example

This example will crash because both parts of the conditional expression will be evaluated even if a is null.

class DangerousNonShortCircuitLogic
{
    public static void Main(string[] args)
    {
        string a = null;
        if (a != null & a.ToLower() == "hello world")
        {
            Console.WriteLine("The string said hello world.");
        }
    }
}

The example is easily fixed by using the short circuit AND operator. The program produces no output but does not crash, unlike the previous example.

class DangerousNonShortCircuitLogicFix
{
    public static void Main(string[] args)
    {
        string a = null;
        if (a != null && a.ToLower() == "hello world")
        {
            Console.WriteLine("The string said hello world.");
        }
    }
}

References

  • © GitHub, Inc.
  • Terms
  • Privacy