hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 17:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Bring qhelp inline with current guidelines

Browse Source
This commit is contained in:
Felicity Chapman
2018-11-08 18:04:31 +00:00
parent 1a5d4626bc
commit 3d779ddebb
11 changed files with 91 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
cpp/ql/src/Critical/DeadCodeCondition.qhelp
View File

@@ -5,23 +5,26 @@
<overview>
<p>This rule finds branching statements with conditions that always evaluate to the same value.
More likely than not these conditions indicate a defect in the branching condition or are an artifact left behind after debugging.</p>
<p>This query finds branching statements with conditions that always evaluate to the same value.
It is likely that these conditions indicate an error in the branching condition.
Alternatively, the conditions may have been left behind after debugging.</p>
<include src="aliasAnalysisWarning.qhelp" />
</overview>
<recommendation>
<p>Check the branch condition for defects, and verify that it isn't a remnant from debugging.</p>
<p>Check the branch condition for logic errors. Check whether it is still required.</p>
</recommendation>
<example><sample src="DeadCodeCondition.cpp" />
<example>
<p>This example shows two branch conditions that always evaluate to the same value.
The two conditions and their associated branches should be deleted.
This will simplify the code and make it easier to maintain.</p>
<sample src="DeadCodeCondition.cpp" />
</example>
<references>
<li>SEI CERT C++ Coding Standard <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.</li>
</references>
</qhelp>

2
cpp/ql/src/Critical/DeadCodeFunction.cpp
View File

@@ -2,7 +2,7 @@ class C {
public:
void g() {
...
//f() was previously used but is now commented, orphaning f()
//f() was previously used but is now commented-out, orphaning f()
//f();
...
}

27
cpp/ql/src/Critical/DeadCodeFunction.qhelp
View File

@@ -3,28 +3,31 @@
"qhelp.dtd">
<qhelp>
<overview>
<p>This rule finds functions that are non-public, non-virtual and are never called. Dead functions are often deprecated pieces of code, and should be removed
as they may increase object code size, decrease code comprehensibility, and create the possibility of misuse.</p>
<p>This query highlights functions that are non-public, non-virtual, and are never called.
Dead functions are often deprecated pieces of code, and should be removed.
If left in the code base they increase object code size, decrease code comprehensibility, and create the possibility of misuse.</p>
<p>
<code>public</code> and <code>protected</code> functions are not considered by the check, as they could be part of the program's
API and could be used by external programs.
<code>public</code> and <code>protected</code> functions are ignored by this query.
This type of function may be part of the program's API and could be used by external programs.
</p>
<include src="callGraphWarning.qhelp" />
</overview>
<recommendation>
<p>Consider removing the function.</p>
<p>Verify that the function is genuinely unused and consider removing it.</p>
</recommendation>
<example><sample src="DeadCodeFunction.cpp" />
<example>
<p>The example below includes a function <code>f</code> that is no longer used and should be deleted.</p>
<sample src="DeadCodeFunction.cpp" />
</example>
<references>
<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.</li>
</references>
</qhelp>

2
cpp/ql/src/Critical/DeadCodeFunction.ql
View File

@@ -1,6 +1,6 @@
/**
* @name Function is never called
* @description A function is never called, and should be considered for removal. Unused functions may increase object size, decrease readability and create the possibility of misuse.
* @description Unused functions may increase object size, decrease readability, and create the possibility of misuse.
* @kind problem
* @id cpp/dead-code-function
* @problem.severity warning

18
cpp/ql/src/Critical/GlobalUseBeforeInit.qhelp
View File

@@ -5,26 +5,26 @@
<overview>
<p>This rule finds calls to functions that use a global variable which happen before the variable was initialized.
<p>This rule finds calls to functions that use a global variable before the variable has been initialized.
Not all compilers generate code that zero-out memory, especially when optimizations are enabled or the compiler
is not compliant with the latest language standards. Accessing uninitialized memory will lead to undefined results.
</p>
<include src="dataFlowWarning.qhelp" />
</overview>
<recommendation>
<p>
Initialize the global variable. If no constant can be used for initialization, ensure that all accesses to the variable occur after
the initialization code is executed.
</p>
</recommendation>
<example><sample src="GlobalUseBeforeInit.cpp" />
<example>
In the example below, <code>callCtr</code> is wrongly used before it has been initialized.
<sample src="GlobalUseBeforeInit.cpp" />
</example>
<references>
<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP53-CPP.+Do+not+read+uninitialized+memory">EXP53-CPP. Do not read uninitialized memory</a>.</li>
</references>
</qhelp>

4
cpp/ql/src/Critical/GlobalUseBeforeInit.ql
View File

@@ -1,6 +1,6 @@
/**
* @name Global variable used before initialization
* @description A function that uses a global variable has been called before the variable has been initialized. Not all compilers zero-out memory for variables, especially when optimizations are enabled, or if the compiler is not compliant with the latest language standards. Using an uninitialized variable leads to undefined results.
* @name Global variable may be used before initialization
* @description Using an uninitialized variable leads to undefined results.
* @kind problem
* @id cpp/global-use-before-init
* @problem.severity warning

27
cpp/ql/src/Critical/InconsistentNullnessTesting.qhelp
View File

@@ -5,23 +5,28 @@
<overview>
<p>This rule finds pointer dereferences that do not check the pointer for nullness, while the same pointer is checked for nullness in other
places in the code. It is most likely that the nullness check was omitted, and that a NULL pointer dereference can occur.
Dereferencing a null pointer and attempting to modify its contents can lead to anything from a segfault to corrupting
important system data (i.e. the interrupt table in some architectures).
<p>This query finds pointer dereferences that do not first check the pointer for nullness,
even though the same pointer is checked for nullness in other
parts of the code. It is likely that the nullness check was accidentally omitted, and that a null pointer dereference can occur.
Dereferencing a null pointer and attempting to modify its contents can lead to anything from a segmentation fault to corrupting
important system data (including the interrupt table in some architectures).
</p>
<include src="pointsToWarning.qhelp" />
</overview>
<recommendation>
<p>Make the nullness check on the pointer consistent across all dereferences.</p>
<p>Use a nullness check consistently in all cases where a pointer is dereferenced.</p>
</recommendation>
<example><sample src="InconsistentNullnessTesting.cpp" />
<example>
This code shows two examples where a pointer is dereferenced.
The first example checks that the pointer is not null before dereferencing it.
The second example fails to perform a nullnes check, leading to a potential vulnerability in the code.
<sample src="InconsistentNullnessTesting.cpp" />
</example>
<references>
<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/MEM10-C.+Define+and+use+a+pointer+validation+function">MEM10-C. Define and use a pointer validation function</a>.</li>
</references>
</qhelp>

2
cpp/ql/src/Critical/InconsistentNullnessTesting.ql
View File

@@ -1,6 +1,6 @@
/**
* @name Inconsistent null check of pointer
* @description A dereferenced pointer is not checked for nullness in the given location, but is checked in other locations. Dereferencing a NULL pointer leads to undefined results.
* @description A dereferenced pointer is not checked for nullness in this location, but it is checked in other locations. Dereferencing a null pointer leads to undefined results.
* @kind problem
* @id cpp/inconsistent-nullness-testing
* @problem.severity warning

28
cpp/ql/src/Critical/OverflowCalculated.qhelp
View File

@@ -6,14 +6,17 @@
<overview>
<p>
This rule finds <code>malloc</code> that use a <code>strlen</code> for the size but to not take the
zero terminator into consideration, and <code>strcat/strncat</code> calls that are done on buffers that do
not have the sufficient size to contain the new string.
This query finds calls to:</p>
<ul>
<li><code>malloc</code> that use a <code>strlen</code> for the buffer size and do not take the
zero terminator into consideration.</li>
<li><code>strcat</code> or <code>strncat</code> that use buffers that are too small to contain the new string.</li>
</ul>
</p>
<p>
The indicated expression will cause a buffer overflow due to a buffer that is of insufficient size to contain
the data being copied. Buffer overflows can result to anything from a segfault to a security vulnerability (particularly
The highlighted expression will cause a buffer overflow because the buffer is too small to contain
the data being copied. Buffer overflows can result to anything from a segmentation fault to a security vulnerability (particularly
if the array is on stack-allocated memory).
</p>
@@ -24,18 +27,23 @@ if the array is on stack-allocated memory).
<p>
Increase the size of the buffer being allocated.
</p>
</recommendation>
<example><sample src="OverflowCalculated.cpp" />
<example>
<p>This example includes thre annotated calls that copy a string into a buffer.
The first call to <code>malloc</code> creates a buffer that's the
same size as the string, leaving no space for the zero terminator
and causing an overflow. The second call to <ocde>malloc</code>
correctly calculates the required buffer size. The call to
<code>strcat</code> appends an additional string to the same buffer
causing a second overflow.</p>
<sample src="OverflowCalculated.cpp" />
</example>
<references>
<references>
<li><a href="http://cwe.mitre.org/data/definitions/131.html">CWE-131: Incorrect Calculation of Buffer Size</a></li>
<li>I. Gerg. <em>An Overview and Example of the Buffer-Overflow Exploit</em>. IANewsletter vol 7 no 4. 2005.</li>
<li>M. Donaldson. <em>Inside the Buffer Overflow Attack: Mechanism, Method &amp; Prevention</em>. SANS Institute InfoSec Reading Room. 2002.</li>
</references>
</qhelp>

22
cpp/ql/src/Critical/OverflowDestination.qhelp
View File

@@ -3,27 +3,31 @@
"qhelp.dtd">
<qhelp>
<overview>
<p>The bounded copy functions <code>memcpy</code>, <code>memmove</code>, <code>strncpy</code>, <code>strncat</code> accept a size argument. You should call these functions with a size argument that is derived from the size of the destination buffer. Using a size argument that is derived from the source buffer may cause a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.</p>
<p>The bounded copy functions <code>memcpy</code>, <code>memmove</code>, <code>strncpy</code>, <code>strncat</code> accept a size argument.
You should call these functions with a size argument that is derived from the size of the destination buffer.
Using a size argument that is derived from the source buffer may cause a buffer overflow.
Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.</p>
</overview>
<recommendation>
<p>Check the highlighted function calls carefully, and ensure that the size parameter is derived from the size of the destination buffer,
<p>Check the highlighted function calls carefully.
Ensure that the size parameter is derived from the size of the destination buffer, and
not the source buffer.</p>
<include src="aliasAnalysisWarning.qhelp" />
</recommendation>
<example><sample src="OverflowDestination.cpp" />
<example>
<p>
The code below shows an example where <code>strncpy</code> is called incorrectly, without checking the size of the destination buffer.
In the second example the call has been updated to include the size of the destination buffer.</p>
<sample src="OverflowDestination.cpp" />
</example>
<references>
<li><a href="http://cwe.mitre.org/data/definitions/119.html">CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer</a></li>
<references>
<li>I. Gerg. <em>An Overview and Example of the Buffer-Overflow Exploit</em>. IANewsletter vol 7 no 4. 2005.</li>
<li>M. Donaldson. <em>Inside the Buffer Overflow Attack: Mechanism, Method &amp; Prevention</em>. SANS Institute InfoSec Reading Room. 2002.</li>
</references>
</qhelp>

2
cpp/ql/src/Critical/OverflowDestination.ql
View File

@@ -1,7 +1,7 @@
/**
* @name Copy function using source size
* @description Calling a copy operation with a size derived from the source
* buffer instead of the destination buffer may result in a buffer overflow
* buffer instead of the destination buffer may result in a buffer overflow.
* @kind problem
* @id cpp/overflow-destination
* @problem.severity warning