hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 17:23:36 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4745 from ihsinme/main

Browse Source 
CPP: Add query for CWE-191 into experimental this reveals a dangerous comparison
This commit is contained in:
Jonas Jensen
2020-12-04 18:00:41 +01:00
committed by GitHub
parent 54d7cac46d 9cf318b72c
commit bc340e210b
3 changed files with 67 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c Normal file
View File

@@ -0,0 +1,11 @@
unsigned long sizeArray;
// BAD: let's consider several values, taking ULONG_MAX =18446744073709551615
// sizeArray = 60; (sizeArray - 10) = 50; true
// sizeArray = 10; (sizeArray - 10) = 0; false
// sizeArray = 1; (sizeArray - 10) = 18446744073709551607; true
// sizeArray = 0; (sizeArray - 10) = 18446744073709551606; true
if (sizeArray - 10 > 0)
// GOOD: Prevent overflow by checking the input
if (sizeArray > 10)

33
cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.qhelp Normal file
View File

@@ -0,0 +1,33 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>The code compares the unsigned difference with zero.
It is highly probable that the condition is wrong if the difference expression has the unsigned type.
The condition holds in all the cases when difference is not equal to zero.
It means that we may use condition not equal. But the programmer probably wanted to compare the difference of elements.</p>
<p>False positives include code in which the first difference element is always greater than or equal to the second.
For comparison, ">" such conditions are equivalent to "! =", And are recommended for replacement.
For comparison "> =", the conditions are always true and are recommended to be excluded.</p>
</overview>
<recommendation>
<p>Use a simple comparison of two elements, instead of comparing their difference to zero.</p>
</recommendation>
<example>
<p>The following example demonstrates an erroneous and corrected use of comparison.</p>
<sample src="UnsignedDifferenceExpressionComparedZero.c" />
</example>
<references>
<li>CERT C Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules">INT02-C. Understand integer conversion rules</a>.
</li>
</references>
</qhelp>

23
cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql Normal file
View File

@@ -0,0 +1,23 @@
/**
* @name Unsigned difference expression compared to zero
* @description It is highly probable that the condition is wrong if the difference expression has the unsigned type.
* The condition holds in all the cases when difference is not equal to zero. It means that we may use condition not equal.
* But the programmer probably wanted to compare the difference of elements.
* @kind problem
* @id cpp/unsigned-difference-expression-compared-zero
* @problem.severity warning
* @precision medium
* @tags security
* external/cwe/cwe-191
*/
import cpp
import semmle.code.cpp.commons.Exclusions
from RelationalOperation ro, SubExpr sub
where
not isFromMacroDefinition(ro) and
ro.getLesserOperand().getValue().toInt() = 0 and
ro.getGreaterOperand() = sub and
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned()
select ro, "Difference in condition is always greater than or equal to zero"