mirror of
https://github.com/github/codeql.git
synced 2026-04-29 18:55:14 +02:00
Update to bring into line with current guidelines
This commit is contained in:
Felicity Chapman
@@ -6,23 +6,25 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule looks at functions that return file or socket descriptors, but may return an error value before actually closing the resource.
|
||||
This can occur when an operation performed on the open descriptor fails, and the function returns with an error before closing the open resource. An improperly handled error could cause the function to leak resource descriptors.
|
||||
This query looks at functions that return file or socket descriptors, but may return an error value before actually closing the resource.
|
||||
This can occur when an operation performed on the open descriptor fails, and the function returns with an error before it closes the open resource. An improperly handled error could cause the function to leak resource descriptors. Failing to close resources in the function that opened them also makes it more difficult to detect leaks.
|
||||
</p>
|
||||
|
||||
<include src="dataFlowWarning.qhelp" />
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Ensure that the function frees all the resources it acquired when an error occurs.</p>
|
||||
|
||||
<p>When an error occurs, ensure that the function frees all the resources it holds open.</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>In the example below, the <code>sockfd</code> socket may remain open if an error is triggered.
|
||||
The code should be updated to ensure that the socket is always closed when when the function ends.
|
||||
</p>
|
||||
<sample src="DescriptorMayNotBeClosed.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/ERR57-CPP.+Do+not+leak+resources+when+handling+exceptions">ERR57-CPP. Do not leak resources when handling exceptions</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Open descriptor may not be closed
|
||||
* @description A function may return before closing a socket or file that was opened in the function. Closing resources in the same function that opened them ties the lifetime of the resource to that of the function call, making it easier to avoid and detect resource leaks.
|
||||
* @description Failing to close resources in the function that opened them, makes it difficult to avoid and detect resource leaks.
|
||||
* @kind problem
|
||||
* @id cpp/descriptor-may-not-be-closed
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -6,20 +6,26 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds calls to <code>open</code> or <code>socket</code> with no corresponding <code>close</code> call in the entire program.
|
||||
This rule finds calls to <code>open</code> or <code>socket</code> where there is no corresponding <code>close</code> call in the program analyzed.
|
||||
Leaving descriptors open will cause a resource leak that will persist even after the program terminates.
|
||||
</p>
|
||||
|
||||
<include src="aliasAnalysisWarning.qhelp" />
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Ensure that all file or socket descriptors allocated by the program are freed before it terminates.</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="DescriptorNeverClosed.cpp" />
|
||||
|
||||
|
||||
<example>
|
||||
<p>In the example below, the <code>sockfd</code> socket remains open when the <code>main</code> program finishes.
|
||||
The code should be updated to ensure that the socket is always closed when the program terminates.
|
||||
</p>
|
||||
|
||||
<sample src="DescriptorNeverClosed.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/ERR57-CPP.+Do+not+leak+resources+when+handling+exceptions">ERR57-CPP. Do not leak resources when handling exceptions</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Open descriptor never closed
|
||||
* @description A function always returns before closing a socket or file that was opened in the function. Closing resources in the same function that opened them ties the lifetime of the resource to that of the function call, making it easier to avoid and detect resource leaks.
|
||||
* @description Functions that always return before closing the socket or file they opened leak resources.
|
||||
* @kind problem
|
||||
* @id cpp/descriptor-never-closed
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -5,23 +5,28 @@
|
||||
|
||||
|
||||
<overview>
|
||||
<p>The rule finds code that initializes a global variable (i.e. uses it as an L-value) but is never called from <code>main</code>.
|
||||
<p>The query finds code that initializes a global variable (that is, uses it as an L-value) but is never called from <code>main</code>.
|
||||
Unless there is another entry point that triggers the initialization, the code should be modified so that the variable is initialized.
|
||||
Uninitialized variables may contain any value, as 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.
|
||||
</p>
|
||||
|
||||
<include src="callGraphWarning.qhelp" />
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Examine the code and ensure that the initialization is always run.</p>
|
||||
<p>Examine the code and ensure that the variable is always initialized before it is used.</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="InitialisationNotRun.cpp" />
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<p>In the example below, the code that triggers the initialization of <code>g_storage</code> is not run from <code>main</code>.
|
||||
Unless the variable is initialized by another method, the call on line 10 may not use the intended value.
|
||||
</p>
|
||||
|
||||
<sample src="InitialisationNotRun.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>C++ reference: <a href="https://en.cppreference.com/book/uninitialized">uninitialized variables</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Initialization code not run
|
||||
* @description A global variable has initialization code, but that code is never run (i.e. is called directly or indirectly from main()). Accessing uninitialized variables leads to undefined results.
|
||||
* @description Using an uninitialized variable may lead to undefined results.
|
||||
* @kind problem
|
||||
* @id cpp/initialization-not-run
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -6,29 +6,36 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds integer values that are first used to index an array and
|
||||
This query finds integer values that are first used to index an array and
|
||||
subsequently tested for being negative. If it is relevant to perform this test
|
||||
at all then it should probably happen <em>before</em> the indexing, not
|
||||
at all then it should happen <em>before</em> the indexing, not
|
||||
after. Otherwise, if the value is negative then the program will have failed
|
||||
before performing the test.
|
||||
</p>
|
||||
<include src="dataFlowWarning.qhelp" />
|
||||
|
||||
<include src="dataFlowWarning.qhelp" />
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
See if the value needs checking before being used as array index. Double-check
|
||||
See if the value needs to be checked before being used as array index. Double-check
|
||||
if the value is derived from user input. If the value clearly cannot be
|
||||
negative then the negativity test is redundant and can be removed.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>The example below includes two functions that use the value <code>recordIdx</code> to
|
||||
index an array and a test to verify that the value is positive. The test is made after
|
||||
<code>printRecord</code> is indexed and before <code>processRecord</code> is indexed.
|
||||
Unless the value of <code>recordIdx</code> cannot be negative, the test should be
|
||||
updated to run <em>before</em> both arrays are indexed.
|
||||
</p>
|
||||
|
||||
<sample src="LateNegativeTest.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>cplusplus.com: <a href="http://www.cplusplus.com/doc/tutorial/pointers/">Pointers</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
/**
|
||||
* @name Pointer offset used before it is checked
|
||||
* @description A value is used as a pointer offset before it is tested for
|
||||
* being positive/negative. This may mean that an unsuitable
|
||||
* pointer offset will be used before the test occurs.
|
||||
* @description Setting a pointer offset before checking if the value is positive
|
||||
* may result in unexpected behavior.
|
||||
* @kind problem
|
||||
* @id cpp/late-negative-test
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -6,33 +6,34 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds pointer arithmetic expressions that use a value returned from a function before the value is checked to be positive.
|
||||
Most pointer arithmetic and almost all array element accesses use a positive value for offsets. A negative value is more likely than not
|
||||
This query finds pointer arithmetic expressions that use a value returned from a function without checking that the value is positive.
|
||||
Most pointer arithmetic and almost all array element accesses use a positive value for offsets. A negative value is likely to be
|
||||
a defect in the returning function. Checking pointer offsets (particularly if they derive from user input) is necessary to avoid
|
||||
buffer overruns.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
The rules only looks at return values of functions that may return a negative value (not all functions).
|
||||
The query looks only at the return values of functions that may return a negative value (not all functions).
|
||||
</p>
|
||||
|
||||
|
||||
<include src="dataFlowWarning.qhelp" />
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Check the function and see whether it needs to check the value to be positive.
|
||||
Review the function. Determine whether it needs to check that the value is positive before performing pointer arithmetic.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>The example below shows an example of this problem. There is no check to ensure that the value of <code>recordIdx</code>
|
||||
is positive and safe to use as an array offset.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<sample src="MissingNegativityTest.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>cplusplus.com: <a href="http://www.cplusplus.com/doc/tutorial/pointers/">Pointers</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
/**
|
||||
* @name Unchecked return value used as offset
|
||||
* @description A return value from a function is used as a pointer offset before it is checked for being positive/negative. Integer values used as pointer offsets should be checked, especially if they are derived from user input.
|
||||
* @description Using a value as a pointer offset without checking that the value is positive
|
||||
* may lead to buffer overruns.
|
||||
* @kind problem
|
||||
* @id cpp/missing-negativity-test
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -6,23 +6,27 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds pointer dereferences that use a pointer returned from a function which may return NULL. Always
|
||||
This query finds pointer dereferences that use a pointer returned from a function which may return NULL. Always
|
||||
check your pointers for NULL-ness before dereferencing them. 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).
|
||||
modify its contents can lead to anything from a segmentation fault to corruption of important system data
|
||||
(for example, the interrupt table in some architectures).
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Add a null check before dereferencing the pointer, or modify the function so that it always returns a non-null value.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="MissingNullTest.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<p>In this example, the function is not protected from dereferencing a null pointer. It should be updated to ensure that
|
||||
this cannot happen.
|
||||
</p>
|
||||
<sample src="MissingNullTest.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointerss">EXP34-C. Do not dereference null pointers</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Returned pointer not checked
|
||||
* @description A value returned from a function that may return null is not tested to determine whether or not it is null. Dereferencing NULL pointers lead to undefined behavior.
|
||||
* @description Dereferencing an untested value from a function that can return null may lead to undefined behavior.
|
||||
* @kind problem
|
||||
* @id cpp/missing-null-test
|
||||
* @problem.severity recommendation
|
||||
|
||||
@@ -6,22 +6,23 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds variables that are used before they are initialized. Values of uninitialized variables are
|
||||
This query finds variables that are used before they are initialized. Values of uninitialized variables are
|
||||
undefined, as not all compilers zero out memory, especially when optimizations are enabled or the compiler
|
||||
is not compliant with the latest language standards.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Initialize the variable before accessing it.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="NotInitialised.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<sample src="NotInitialised.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>C++ reference: <a href="https://en.cppreference.com/book/uninitialized">uninitialized variables</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Variable not initialized before use
|
||||
* @description A variable is used before initialized. The value of a variable is undefined before initialization, and its use should be avoided.
|
||||
* @description Using an uninitialized variable may lead to undefined results.
|
||||
* @kind problem
|
||||
* @id cpp/not-initialised
|
||||
* @problem.severity error
|
||||
|
||||
@@ -6,27 +6,31 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds return statements that return pointers to an object allocated on the stack.
|
||||
This query finds return statements that return pointers to an object allocated on the stack.
|
||||
The lifetime of a stack allocated memory location only lasts until the function returns, and
|
||||
the contents of that memory become undefined after that. Clearly, using a pointer to stack
|
||||
memory after the function has already returned will have undefined results.
|
||||
</p>
|
||||
|
||||
<include src="pointsToWarning.qhelp" />
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Do not return pointers to stack memory locations. Instead, create an output parameter, or create a heap-allocated
|
||||
buffer, copy the contents of the stack allocated memory to that buffer and return that instead.
|
||||
buffer. You can then copy the contents of the stack-allocated memory to the heap-allocated buffer and return that location instead.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="ReturnStackAllocatedObject.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<p>The example below the reference to <code>myRecord</code> is useful only while the containing function is running.
|
||||
If you need to access the object outside this function, either create an output parameter with its value, or copy the object into
|
||||
heap-allocated memory.
|
||||
</p>
|
||||
<sample src="ReturnStackAllocatedObject.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>cplusplus.com: <a href="http://www.cplusplus.com/doc/tutorial/pointers/">Pointers</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,10 +1,6 @@
|
||||
/**
|
||||
* @name Pointer to stack object used as return value
|
||||
* @description A function has returned a pointer to an object allocated on
|
||||
* the stack. The lifetime of stack allocated memory ends when the
|
||||
* stack frame of the function that allocated it is popped off the
|
||||
* stack. Any pointer to memory in a function call's stack frame
|
||||
* will be a dangling pointer after the function returns.
|
||||
* @description Using a pointer to stack memory after the function has returned gives undefined results.
|
||||
* @kind problem
|
||||
* @id cpp/return-stack-allocated-object
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -6,23 +6,24 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds variables that are assigned a value but are never read. This is usually an indication of a variable that has been orphaned
|
||||
This query finds variables that are assigned a value but are never read. This is usually an indication of a variable that has been orphaned
|
||||
due to changes in code, or a defect in the code due to the omission of the unused variable. The unused variables should be
|
||||
removed to avoid misuse.
|
||||
</p>
|
||||
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Examine the code to see if the variable should really be unused, and remove it if it is.
|
||||
Examine the code to see if the variable is no longer needed. If it is unnecessary, remove the variable.
|
||||
Otherwise, update the relevant code to use the variable.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="Unused.cpp" />
|
||||
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<sample src="Unused.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC13-C.+Detect+and+remove+unused+values">MSC13-C. Detect and remove unused values</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Variable is assigned a value that is never read
|
||||
* @description A variable is assigned a value but is never read. These variables are usually orphaned due to changes in code and can be removed, or may indicate a bug in the code that is caused by an omission of the unused variable.
|
||||
* @description Assigning a value to a variable that is not used may indicate an error in the code.
|
||||
* @kind problem
|
||||
* @id cpp/unused-variable
|
||||
* @problem.severity warning
|
||||
|
||||
@@ -8,7 +8,7 @@ public:
|
||||
MyClass &operator=(const MyClass &other)
|
||||
{
|
||||
delete data;
|
||||
data = other.data->clone(); // BAD: if other == *this, other.data has already been deleted!
|
||||
data = other.data->clone(); // wrong: if other == *this, other.data has already been deleted!
|
||||
return *this;
|
||||
}
|
||||
|
||||
@@ -28,7 +28,7 @@ public:
|
||||
|
||||
MyClass &operator=(const MyClass &other)
|
||||
{
|
||||
if (this == &other) { return *this; } // FIXED
|
||||
if (this == &other) { return *this; } // correct
|
||||
delete data;
|
||||
data = other.data->clone();
|
||||
return *this;
|
||||
|
||||
@@ -12,17 +12,19 @@ This could lead to accessing an already freed memory location.</p>
|
||||
before getting a new value from the object on the right hand side.</p>
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Copy assignment operator should check for self-assignment.</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="SelfAssignmentCheck.cpp" />
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<p>This example shows a copy assignment operator that fails to check for self assignment.
|
||||
The corrected version of the same operator is also included.
|
||||
</p>
|
||||
<sample src="SelfAssignmentCheck.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
|
||||
<li>[1] M. Cline, <a href="https://isocpp.org/wiki/faq/assignment-operators">Part of C++ FAQ: Assignment operators</a></li>
|
||||
|
||||
</references></qhelp>
|
||||
<li>Standard C++ Foundation: <a href="https://isocpp.org/wiki/faq/assignment-operators">Assignment Operators</a></li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
struct {
|
||||
short s : 4; //wrong: behavior of signed bit-field members vary across compilers
|
||||
short s : 4; //wrong: behavior of signed bit-field members varies across compilers
|
||||
unsigned int : 24; //correct: unsigned
|
||||
} bits;
|
||||
|
||||
@@ -6,34 +6,31 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds bit fields with members that are not explicitly declared to be unsigned.
|
||||
This query finds bit fields with members that are not explicitly declared to be unsigned.
|
||||
The sign of plain char, short, int, or long bit field is implementation-specific, and declaring
|
||||
them all to be unsigned removes the ambiguity and ensures portability.
|
||||
</p>
|
||||
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Declare all members of the bit field to be unsigned.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="AV Rule 154.cpp" />
|
||||
|
||||
|
||||
|
||||
<example>
|
||||
<p>The code below shows two examples of bit fields. The second field is declared to be unsigned which ensures portability.
|
||||
The first field should also be declared to be unsigned.
|
||||
</p>
|
||||
<sample src="AV Rule 154.cpp" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
|
||||
|
||||
<li>
|
||||
AV Rule 154, <em>Joint Strike Fighter Air Vehicle C++ Coding Standards</em>. Lockheed Martin Corporation, 2005.
|
||||
</li>
|
||||
<li>
|
||||
<a href="http://en.cppreference.com/w/cpp/language/bit_field">C++ Bit Fields</a>
|
||||
C++ reference: <a href="http://en.cppreference.com/w/cpp/language/bit_field">Bit Fields</a>
|
||||
</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,10 +1,7 @@
|
||||
/**
|
||||
* @name Possible signed bit-field member
|
||||
* @description Bit fields should have explicitly unsigned integral or
|
||||
* enumeration types only. For example, use `unsigned int` rather
|
||||
* than `int`. It is implementation specific whether an
|
||||
* `int`-typed bit field is signed, so there could be unexpected
|
||||
* sign extension or overflow.
|
||||
* @description Failing to explicitly assign bit fields to unsigned integer or enumeration types
|
||||
* may result in unexpected sign extension or overflow.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @precision low
|
||||
|
||||
Reference in New Issue
Block a user