Potentially dangerous use of non-short-circuit logic¶
ID: cs/non-short-circuitKind: problemSecurity severity: Severity: errorPrecision: highTags: - quality - reliability - correctness - external/cwe/cwe-480 - external/cwe/cwe-691Query 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 ifa is null.
classDangerousNonShortCircuitLogic{publicstaticvoidMain(string[]args){stringa=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.
classDangerousNonShortCircuitLogicFix{publicstaticvoidMain(string[]args){stringa=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.