diff --git a/cpp/ql/lib/semmle/code/cpp/Function.qll b/cpp/ql/lib/semmle/code/cpp/Function.qll index 0648c6260ce5..7cfa779338f7 100644 --- a/cpp/ql/lib/semmle/code/cpp/Function.qll +++ b/cpp/ql/lib/semmle/code/cpp/Function.qll @@ -500,6 +500,17 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function { * Gets the nearest enclosing AccessHolder. */ override AccessHolder getEnclosingAccessHolder() { result = this.getDeclaringType() } + + /** + * Holds if this function has extraction errors that create an `ErrorExpr`. + */ + predicate hasErrors() { + exists(ErrorExpr e | + e.getEnclosingFunction() = this and + // Exclude the first allocator call argument because it is always extracted as `ErrorExpr`. + not exists(NewOrNewArrayExpr new | e = new.getAllocatorCall().getArgument(0)) + ) + } } pragma[noinline] diff --git a/cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql b/cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql index 1d02474bfbb7..5ae8468d0fc4 100644 --- a/cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql +++ b/cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql @@ -57,5 +57,5 @@ where not declarationHasSideEffects(v) and not exists(AsmStmt s | f = s.getEnclosingFunction()) and not v.getAnAttribute().getName() = "unused" and - not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr may use `v` + not f.hasErrors() // Unextracted expressions may use `v` select v, "Variable " + v.getName() + " is not used." diff --git a/cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql b/cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql index 16679d67fd2e..02678beaf124 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql +++ b/cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql @@ -29,7 +29,7 @@ class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration { override predicate isSource(Instruction source) { exists(Function func | // Rule out FPs caused by extraction errors. - not any(ErrorExpr e).getEnclosingFunction() = func and + not func.hasErrors() and not intentionallyReturnsStackPointer(func) and func = source.getEnclosingFunction() | diff --git a/cpp/ql/src/Likely Bugs/Memory Management/UninitializedLocal.ql b/cpp/ql/src/Likely Bugs/Memory Management/UninitializedLocal.ql index 35bee25c9f5f..763a142f1b90 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/UninitializedLocal.ql +++ b/cpp/ql/src/Likely Bugs/Memory Management/UninitializedLocal.ql @@ -65,6 +65,7 @@ predicate isSinkImpl(Instruction sink, VariableAccess va) { exists(LoadInstruction load | va = load.getUnconvertedResultExpression() and not va = commonException() and + not va.getTarget().(LocalVariable).getFunction().hasErrors() and sink = load.getSourceValue() ) } diff --git a/cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql b/cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql index 678cb95a7214..0df59b5f01db 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql +++ b/cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql @@ -24,7 +24,7 @@ predicate instructionHasVariable(VariableAddressInstruction vai, StackVariable v // Pointer-to-member types aren't properly handled in the dbscheme. not vai.getResultType() instanceof PointerToMemberType and // Rule out FPs caused by extraction errors. - not any(ErrorExpr e).getEnclosingFunction() = f + not f.hasErrors() } /** diff --git a/cpp/ql/src/change-notes/2024-10-02-uninitialized-local.md b/cpp/ql/src/change-notes/2024-10-02-uninitialized-local.md new file mode 100644 index 000000000000..e34a942f38a6 --- /dev/null +++ b/cpp/ql/src/change-notes/2024-10-02-uninitialized-local.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Fixed false positives in the `cpp/uninitialized-local` ("Potentially uninitialized local variable") query if there are extraction errors in the function. diff --git a/cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql b/cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql index ac5db25ea6b5..9027c064ac67 100644 --- a/cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql +++ b/cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql @@ -49,7 +49,7 @@ predicate functionsMissingReturnStmt(Function f, ControlFlowNode blame) { predicate functionImperfectlyExtracted(Function f) { exists(CompilerError e | f.getBlock().getLocation().subsumes(e.getLocation())) or - exists(ErrorExpr ee | ee.getEnclosingFunction() = f) + f.hasErrors() or count(f.getType()) > 1 or diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/UninitializedLocal.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/UninitializedLocal.expected index a8b3c7782e7c..6773f5aef942 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/UninitializedLocal.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/UninitializedLocal.expected @@ -1,5 +1,6 @@ edges nodes +| errors.cpp:13:7:13:7 | definition of x | semmle.label | definition of x | | test.cpp:11:6:11:8 | definition of foo | semmle.label | definition of foo | | test.cpp:111:6:111:8 | definition of foo | semmle.label | definition of foo | | test.cpp:226:7:226:7 | definition of x | semmle.label | definition of x | @@ -14,6 +15,7 @@ nodes | test.cpp:472:6:472:6 | definition of x | semmle.label | definition of x | | test.cpp:479:6:479:6 | definition of x | semmle.label | definition of x | #select +| errors.cpp:14:18:14:18 | x | errors.cpp:13:7:13:7 | definition of x | errors.cpp:13:7:13:7 | definition of x | The variable $@ may not be initialized at this access. | errors.cpp:13:7:13:7 | x | x | | test.cpp:12:6:12:8 | foo | test.cpp:11:6:11:8 | definition of foo | test.cpp:11:6:11:8 | definition of foo | The variable $@ may not be initialized at this access. | test.cpp:11:6:11:8 | foo | foo | | test.cpp:113:6:113:8 | foo | test.cpp:111:6:111:8 | definition of foo | test.cpp:111:6:111:8 | definition of foo | The variable $@ may not be initialized at this access. | test.cpp:111:6:111:8 | foo | foo | | test.cpp:227:3:227:3 | x | test.cpp:226:7:226:7 | definition of x | test.cpp:226:7:226:7 | definition of x | The variable $@ may not be initialized at this access. | test.cpp:226:7:226:7 | x | x | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/errors.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/errors.cpp new file mode 100644 index 000000000000..07bb61f943ed --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/errors.cpp @@ -0,0 +1,15 @@ +// semmle-extractor-options: --expect_errors + +int f1() { + int x; + initialize(&x); // error expression - initialize() is not defined + return x; // GOOD - assume x is initialized +} + +void * operator new(unsigned long, bool); +void operator delete(void*, bool); + +int f2() { + int x; + new(true) int (x); // BAD, ignore implicit error expression +}