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¶
MSDN: & Operator
MSDN: | Operator
Common Weakness Enumeration: CWE-480.
Common Weakness Enumeration: CWE-691.