From 98b97b09be3ac7aa0a9d67f61ee5a40da84e6ea7 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 11 Jul 2019 13:48:51 -0700 Subject: [PATCH 1/8] C++: add hasGlobalOrStdName to Declaration --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index fa061b239cc4..55d9a39dbdc4 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -119,6 +119,11 @@ abstract class Declaration extends Locatable, @declaration { /** Holds if this declaration has the given name in the global namespace. */ predicate hasGlobalName(string name) { this.hasQualifiedName("", "", name) } + /** Holds if this declaration has the given name in the global namespace or the `std` namespace. */ + predicate hasGlobalOrStdName(string name) { + this.hasQualifiedName("std", "", name) + } + /** Gets a specifier of this declaration. */ abstract Specifier getASpecifier(); From bff68a00ac074a08fce4112a6f59288a440f09f5 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 11 Jul 2019 15:57:26 -0700 Subject: [PATCH 2/8] C++: Add Declaration.hasStdName --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 55d9a39dbdc4..7c38acc564ee 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -119,9 +119,17 @@ abstract class Declaration extends Locatable, @declaration { /** Holds if this declaration has the given name in the global namespace. */ predicate hasGlobalName(string name) { this.hasQualifiedName("", "", name) } + + /** Holds if this declaration has the given name in the `std` namespace. */ + predicate hasStdName(string name) { + this.hasQualifiedName("std", "", name) + } + /** Holds if this declaration has the given name in the global namespace or the `std` namespace. */ predicate hasGlobalOrStdName(string name) { - this.hasQualifiedName("std", "", name) + this.hasGlobalName(name) + or + this.hasStdName(name) } /** Gets a specifier of this declaration. */ From 03f72d207cbcc02cedc5973d9ffcb0cd2f00ea00 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 11 Jul 2019 16:00:41 -0700 Subject: [PATCH 3/8] C++: use Declaration.hasGlobalOrStdName --- .../src/Critical/DescriptorMayNotBeClosed.ql | 2 +- cpp/ql/src/Critical/DescriptorNeverClosed.ql | 2 +- cpp/ql/src/Critical/MemoryMayNotBeFreed.ql | 4 +- cpp/ql/src/Critical/OverflowCalculated.ql | 9 ++- cpp/ql/src/Critical/OverflowDestination.ql | 2 +- cpp/ql/src/Critical/OverflowStatic.ql | 16 ++--- cpp/ql/src/Critical/SizeCheck.ql | 4 +- cpp/ql/src/Critical/SizeCheck2.ql | 4 +- cpp/ql/src/Critical/UseAfterFree.ql | 2 +- cpp/ql/src/DefaultOptions.qll | 4 +- .../src/Security/CWE/CWE-022/TaintedPath.ql | 7 ++- cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql | 4 +- .../CWE/CWE-121/UnterminatedVarargsCall.ql | 2 +- .../CWE/CWE-131/NoSpaceForZeroTerminator.ql | 5 +- .../Security/CWE/CWE-497/ExposedSystemData.ql | 60 +++++++++---------- .../CWE/CWE-676/DangerousFunctionOverflow.ql | 2 +- cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql | 4 +- cpp/ql/src/jsf/4.10 Classes/AV Rule 97.ql | 4 +- cpp/ql/src/semmle/code/cpp/Function.qll | 2 +- cpp/ql/src/semmle/code/cpp/commons/Alloc.qll | 16 +++-- .../semmle/code/cpp/commons/Environment.qll | 2 +- cpp/ql/src/semmle/code/cpp/commons/File.qll | 12 ++-- .../code/cpp/commons/StringAnalysis.qll | 4 +- .../code/cpp/controlflow/Dereferenced.qll | 2 +- .../semmle/code/cpp/controlflow/Nullness.qll | 4 +- .../cpp/models/implementations/Printf.qll | 22 +++---- .../code/cpp/models/implementations/Pure.qll | 4 +- .../code/cpp/security/CommandExecution.qll | 2 +- .../semmle/code/cpp/security/FileWrite.qll | 2 +- .../semmle/code/cpp/security/OutputWrite.qll | 4 +- .../src/semmle/code/cpp/security/Security.qll | 14 +++-- .../code/cpp/security/TaintTracking.qll | 36 +++++------ 32 files changed, 138 insertions(+), 125 deletions(-) diff --git a/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql b/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql index 9c93e066119c..47401c6eea57 100644 --- a/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql +++ b/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql @@ -13,7 +13,7 @@ import semmle.code.cpp.pointsto.PointsTo import Negativity predicate closeCall(FunctionCall fc, Variable v) { - fc.getTarget().hasGlobalName("close") and v.getAnAccess() = fc.getArgument(0) + fc.getTarget().hasGlobalOrStdName("close") and v.getAnAccess() = fc.getArgument(0) or exists(FunctionCall midcall, Function mid, int arg | fc.getArgument(arg) = v.getAnAccess() and diff --git a/cpp/ql/src/Critical/DescriptorNeverClosed.ql b/cpp/ql/src/Critical/DescriptorNeverClosed.ql index b52af1bf2a3d..f06708f4ae36 100644 --- a/cpp/ql/src/Critical/DescriptorNeverClosed.ql +++ b/cpp/ql/src/Critical/DescriptorNeverClosed.ql @@ -13,7 +13,7 @@ import semmle.code.cpp.pointsto.PointsTo predicate closed(Expr e) { exists(FunctionCall fc | - fc.getTarget().hasGlobalName("close") and + fc.getTarget().hasGlobalOrStdName("close") and fc.getArgument(0) = e ) } diff --git a/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql b/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql index 9bba8e9896d6..2bede6819126 100644 --- a/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql +++ b/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql @@ -53,7 +53,7 @@ predicate allocCallOrIndirect(Expr e) { * can cause memory leaks. */ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode verified) { - reallocCall.getTarget().hasGlobalName("realloc") and + reallocCall.getTarget().hasGlobalOrStdName("realloc") and reallocCall.getArgument(0) = v.getAnAccess() and ( exists(Variable newV, ControlFlowNode node | @@ -79,7 +79,7 @@ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode predicate freeCallOrIndirect(ControlFlowNode n, Variable v) { // direct free call freeCall(n, v.getAnAccess()) and - not n.(FunctionCall).getTarget().hasGlobalName("realloc") + not n.(FunctionCall).getTarget().hasGlobalOrStdName("realloc") or // verified realloc call verifiedRealloc(_, v, n) diff --git a/cpp/ql/src/Critical/OverflowCalculated.ql b/cpp/ql/src/Critical/OverflowCalculated.ql index 36ee0140cf7e..c528be8ca242 100644 --- a/cpp/ql/src/Critical/OverflowCalculated.ql +++ b/cpp/ql/src/Critical/OverflowCalculated.ql @@ -14,8 +14,7 @@ import cpp class MallocCall extends FunctionCall { MallocCall() { - this.getTarget().hasGlobalName("malloc") or - this.getTarget().hasQualifiedName("std", "malloc") + this.getTarget().hasGlobalOrStdName("malloc") } Expr getAllocatedSize() { @@ -36,12 +35,12 @@ predicate spaceProblem(FunctionCall append, string msg) { malloc.getAllocatedSize() = add and buffer.getAnAccess() = strlen.getStringExpr() and ( - insert.getTarget().hasGlobalName("strcpy") or + insert.getTarget().hasGlobalOrStdName("strcpy") or insert.getTarget().hasGlobalName("strncpy") ) and ( - append.getTarget().hasGlobalName("strcat") or - append.getTarget().hasGlobalName("strncat") + append.getTarget().hasGlobalOrStdName("strcat") or + append.getTarget().hasGlobalOrStdName("strncat") ) and malloc.getASuccessor+() = insert and insert.getArgument(1) = buffer.getAnAccess() and diff --git a/cpp/ql/src/Critical/OverflowDestination.ql b/cpp/ql/src/Critical/OverflowDestination.ql index d7b02b5d8d58..ad925daed628 100644 --- a/cpp/ql/src/Critical/OverflowDestination.ql +++ b/cpp/ql/src/Critical/OverflowDestination.ql @@ -25,7 +25,7 @@ import semmle.code.cpp.security.TaintTracking predicate sourceSized(FunctionCall fc, Expr src) { exists(string name | (name = "strncpy" or name = "strncat" or name = "memcpy" or name = "memmove") and - fc.getTarget().hasGlobalName(name) + fc.getTarget().hasGlobalOrStdName(name) ) and exists(Expr dest, Expr size, Variable v | fc.getArgument(0) = dest and diff --git a/cpp/ql/src/Critical/OverflowStatic.ql b/cpp/ql/src/Critical/OverflowStatic.ql index 1892d5acff1f..8cee25007972 100644 --- a/cpp/ql/src/Critical/OverflowStatic.ql +++ b/cpp/ql/src/Critical/OverflowStatic.ql @@ -58,21 +58,21 @@ predicate overflowOffsetInLoop(BufferAccess bufaccess, string msg) { } predicate bufferAndSizeFunction(Function f, int buf, int size) { - f.hasGlobalName("read") and buf = 1 and size = 2 + f.hasGlobalOrStdName("read") and buf = 1 and size = 2 or - f.hasGlobalName("fgets") and buf = 0 and size = 1 + f.hasGlobalOrStdName("fgets") and buf = 0 and size = 1 or - f.hasGlobalName("strncpy") and buf = 0 and size = 2 + f.hasGlobalOrStdName("strncpy") and buf = 0 and size = 2 or - f.hasGlobalName("strncat") and buf = 0 and size = 2 + f.hasGlobalOrStdName("strncat") and buf = 0 and size = 2 or - f.hasGlobalName("memcpy") and buf = 0 and size = 2 + f.hasGlobalOrStdName("memcpy") and buf = 0 and size = 2 or - f.hasGlobalName("memmove") and buf = 0 and size = 2 + f.hasGlobalOrStdName("memmove") and buf = 0 and size = 2 or - f.hasGlobalName("snprintf") and buf = 0 and size = 1 + f.hasGlobalOrStdName("snprintf") and buf = 0 and size = 1 or - f.hasGlobalName("vsnprintf") and buf = 0 and size = 1 + f.hasGlobalOrStdName("vsnprintf") and buf = 0 and size = 1 } class CallWithBufferSize extends FunctionCall { diff --git a/cpp/ql/src/Critical/SizeCheck.ql b/cpp/ql/src/Critical/SizeCheck.ql index da841b73c9bc..313763ba56cc 100644 --- a/cpp/ql/src/Critical/SizeCheck.ql +++ b/cpp/ql/src/Critical/SizeCheck.ql @@ -17,12 +17,12 @@ import cpp class Allocation extends FunctionCall { Allocation() { exists(string name | - this.getTarget().hasGlobalName(name) and + this.getTarget().hasGlobalOrStdName(name) and (name = "malloc" or name = "calloc" or name = "realloc") ) } - private string getName() { this.getTarget().hasGlobalName(result) } + private string getName() { this.getTarget().hasGlobalOrStdName(result) } int getSize() { this.getName() = "malloc" and diff --git a/cpp/ql/src/Critical/SizeCheck2.ql b/cpp/ql/src/Critical/SizeCheck2.ql index 3cb5d1d28b00..1b716d79d49e 100644 --- a/cpp/ql/src/Critical/SizeCheck2.ql +++ b/cpp/ql/src/Critical/SizeCheck2.ql @@ -17,12 +17,12 @@ import cpp class Allocation extends FunctionCall { Allocation() { exists(string name | - this.getTarget().hasGlobalName(name) and + this.getTarget().hasGlobalOrStdName(name) and (name = "malloc" or name = "calloc" or name = "realloc") ) } - private string getName() { this.getTarget().hasGlobalName(result) } + private string getName() { this.getTarget().hasGlobalOrStdName(result) } int getSize() { this.getName() = "malloc" and diff --git a/cpp/ql/src/Critical/UseAfterFree.ql b/cpp/ql/src/Critical/UseAfterFree.ql index db78c206ea1a..9efbb6c3b447 100644 --- a/cpp/ql/src/Critical/UseAfterFree.ql +++ b/cpp/ql/src/Critical/UseAfterFree.ql @@ -16,7 +16,7 @@ import semmle.code.cpp.controlflow.LocalScopeVariableReachability predicate isFreeExpr(Expr e, LocalScopeVariable v) { exists(VariableAccess va | va.getTarget() = v | exists(FunctionCall fc | fc = e | - fc.getTarget().hasGlobalName("free") and + fc.getTarget().hasGlobalOrStdName("free") and va = fc.getArgument(0) ) or diff --git a/cpp/ql/src/DefaultOptions.qll b/cpp/ql/src/DefaultOptions.qll index 27e5584369e2..3e03ec9ee659 100644 --- a/cpp/ql/src/DefaultOptions.qll +++ b/cpp/ql/src/DefaultOptions.qll @@ -59,7 +59,7 @@ class Options extends string { predicate exits(Function f) { f.getAnAttribute().hasName("noreturn") or - exists(string name | f.hasGlobalName(name) | + exists(string name | f.hasGlobalOrStdName(name) | name = "exit" or name = "_exit" or name = "abort" or @@ -91,7 +91,7 @@ class Options extends string { * By default holds only for `fgets`. */ predicate alwaysCheckReturnValue(Function f) { - f.hasGlobalName("fgets") or + f.hasGlobalOrStdName("fgets") or CustomOptions::alwaysCheckReturnValue(f) // old Options.qll } diff --git a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql index 096b1468bb9a..fba0ddda4a13 100644 --- a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -34,8 +34,13 @@ class FileFunction extends FunctionWithWrappers { nme.matches("CreateFile%") ) or + exists(string nme | this.hasStdName(nme) | + nme = "fopen" or + nme = "open" + ) + or // on any of the fstream classes, or filebuf - exists(string nme | this.getDeclaringType().getSimpleName() = nme | + exists(string nme | this.getDeclaringType().hasStdName(nme) | nme = "basic_fstream" or nme = "basic_ifstream" or nme = "basic_ofstream" or diff --git a/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql b/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql index 4469517c369b..8b7fb83df81d 100644 --- a/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql +++ b/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql @@ -17,8 +17,8 @@ import semmle.code.cpp.security.TaintTracking /** A call that prints its arguments to `stdout`. */ class PrintStdoutCall extends FunctionCall { PrintStdoutCall() { - getTarget().hasGlobalName("puts") or - getTarget().hasGlobalName("printf") + getTarget().hasGlobalOrStdName("puts") or + getTarget().hasGlobalOrStdName("printf") } } diff --git a/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql b/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql index 473f0f72f122..77225ad05776 100644 --- a/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql +++ b/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql @@ -66,7 +66,7 @@ class VarargsFunction extends Function { } predicate isWhitelisted() { - this.hasGlobalName("open") or + this.hasGlobalOrStdName("open") or this.hasGlobalName("fcntl") or this.hasGlobalName("ptrace") } diff --git a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql index 575752f0b74e..ff17daca05c3 100644 --- a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql +++ b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql @@ -19,10 +19,7 @@ import semmle.code.cpp.dataflow.DataFlow import semmle.code.cpp.models.implementations.Memcpy class MallocCall extends FunctionCall { - MallocCall() { - this.getTarget().hasGlobalName("malloc") or - this.getTarget().hasQualifiedName("std", "malloc") - } + MallocCall() { this.getTarget().hasGlobalOrStdName("malloc") } Expr getAllocatedSize() { if this.getArgument(0) instanceof VariableAccess diff --git a/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql b/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql index fa74c85555d7..63eca2922977 100644 --- a/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql +++ b/cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql @@ -190,11 +190,11 @@ private predicate windowsSystemInfo(FunctionCall source, Element use) { // void WINAPI GetSystemInfo(_Out_ LPSYSTEM_INFO lpSystemInfo); // void WINAPI GetNativeSystemInfo(_Out_ LPSYSTEM_INFO lpSystemInfo); ( - source.getTarget().hasName("GetVersionEx") or - source.getTarget().hasName("GetVersionExA") or - source.getTarget().hasName("GetVersionExW") or - source.getTarget().hasName("GetSystemInfo") or - source.getTarget().hasName("GetNativeSystemInfo") + source.getTarget().hasGlobalName("GetVersionEx") or + source.getTarget().hasGlobalName("GetVersionExA") or + source.getTarget().hasGlobalName("GetVersionExW") or + source.getTarget().hasGlobalName("GetSystemInfo") or + source.getTarget().hasGlobalName("GetNativeSystemInfo") ) and use = source.getArgument(0) } @@ -216,9 +216,9 @@ private predicate windowsFolderPath(FunctionCall source, Element use) { // _In_ BOOL fCreate // ); ( - source.getTarget().hasName("SHGetSpecialFolderPath") or - source.getTarget().hasName("SHGetSpecialFolderPathA") or - source.getTarget().hasName("SHGetSpecialFolderPathW") + source.getTarget().hasGlobalName("SHGetSpecialFolderPath") or + source.getTarget().hasGlobalName("SHGetSpecialFolderPathA") or + source.getTarget().hasGlobalName("SHGetSpecialFolderPathW") ) and use = source.getArgument(1) or @@ -228,7 +228,7 @@ private predicate windowsFolderPath(FunctionCall source, Element use) { // _In_opt_ HANDLE hToken, // _Out_ PWSTR *ppszPath // ); - source.getTarget().hasName("SHGetKnownFolderPath") and + source.getTarget().hasGlobalName("SHGetKnownFolderPath") and use = source.getArgument(3) or // HRESULT SHGetFolderPath( @@ -239,9 +239,9 @@ private predicate windowsFolderPath(FunctionCall source, Element use) { // _Out_ LPTSTR pszPath // ); ( - source.getTarget().hasName("SHGetFolderPath") or - source.getTarget().hasName("SHGetFolderPathA") or - source.getTarget().hasName("SHGetFolderPathW") + source.getTarget().hasGlobalName("SHGetFolderPath") or + source.getTarget().hasGlobalName("SHGetFolderPathA") or + source.getTarget().hasGlobalName("SHGetFolderPathW") ) and use = source.getArgument(4) or @@ -254,9 +254,9 @@ private predicate windowsFolderPath(FunctionCall source, Element use) { // _Out_ LPTSTR pszPath // ); ( - source.getTarget().hasName("SHGetFolderPathAndSubDir") or - source.getTarget().hasName("SHGetFolderPathAndSubDirA") or - source.getTarget().hasName("SHGetFolderPathAndSubDirW") + source.getTarget().hasGlobalName("SHGetFolderPathAndSubDir") or + source.getTarget().hasGlobalName("SHGetFolderPathAndSubDirA") or + source.getTarget().hasGlobalName("SHGetFolderPathAndSubDirW") ) and use = source.getArgument(5) } @@ -273,9 +273,9 @@ class WindowsFolderPath extends SystemData { private predicate logonUser(FunctionCall source, VariableAccess use) { ( - source.getTarget().hasName("LogonUser") or - source.getTarget().hasName("LogonUserW") or - source.getTarget().hasName("LogonUserA") + source.getTarget().hasGlobalName("LogonUser") or + source.getTarget().hasGlobalName("LogonUserW") or + source.getTarget().hasGlobalName("LogonUserA") ) and use = source.getAnArgument() } @@ -297,9 +297,9 @@ private predicate regQuery(FunctionCall source, VariableAccess use) { // _Inout_opt_ PLONG lpcbValue // ); ( - source.getTarget().hasName("RegQueryValue") or - source.getTarget().hasName("RegQueryValueA") or - source.getTarget().hasName("RegQueryValueW") + source.getTarget().hasGlobalName("RegQueryValue") or + source.getTarget().hasGlobalName("RegQueryValueA") or + source.getTarget().hasGlobalName("RegQueryValueW") ) and use = source.getArgument(2) or @@ -311,9 +311,9 @@ private predicate regQuery(FunctionCall source, VariableAccess use) { // _Inout_opt_ LPDWORD ldwTotsize // ); ( - source.getTarget().hasName("RegQueryMultipleValues") or - source.getTarget().hasName("RegQueryMultipleValuesA") or - source.getTarget().hasName("RegQueryMultipleValuesW") + source.getTarget().hasGlobalName("RegQueryMultipleValues") or + source.getTarget().hasGlobalName("RegQueryMultipleValuesA") or + source.getTarget().hasGlobalName("RegQueryMultipleValuesW") ) and use = source.getArgument(3) or @@ -326,9 +326,9 @@ private predicate regQuery(FunctionCall source, VariableAccess use) { // _Inout_opt_ LPDWORD lpcbData // ); ( - source.getTarget().hasName("RegQueryValueEx") or - source.getTarget().hasName("RegQueryValueExA") or - source.getTarget().hasName("RegQueryValueExW") + source.getTarget().hasGlobalName("RegQueryValueEx") or + source.getTarget().hasGlobalName("RegQueryValueExA") or + source.getTarget().hasGlobalName("RegQueryValueExW") ) and use = source.getArgument(4) or @@ -342,9 +342,9 @@ private predicate regQuery(FunctionCall source, VariableAccess use) { // _Inout_opt_ LPDWORD pcbData // ); ( - source.getTarget().hasName("RegGetValue") or - source.getTarget().hasName("RegGetValueA") or - source.getTarget().hasName("RegGetValueW") + source.getTarget().hasGlobalName("RegGetValue") or + source.getTarget().hasGlobalName("RegGetValueA") or + source.getTarget().hasGlobalName("RegGetValueW") ) and use = source.getArgument(5) } diff --git a/cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql b/cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql index 84d7b48e265a..fe9b56bd5217 100644 --- a/cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql +++ b/cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql @@ -15,5 +15,5 @@ import cpp from FunctionCall call, Function target where call.getTarget() = target and - target.hasGlobalName("gets") + target.hasGlobalOrStdName("gets") select call, "gets does not guard against buffer overflow" diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql index 4b1f45185d17..f2512b93003f 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql @@ -22,7 +22,7 @@ predicate acquireExpr(Expr acquire, string kind) { exists(FunctionCall fc, Function f, string name | fc = acquire and f = fc.getTarget() and - f.hasGlobalName(name) and + f.hasGlobalOrStdName(name) and ( name = "fopen" and kind = "file" @@ -46,7 +46,7 @@ predicate releaseExpr(Expr release, Expr resource, string kind) { exists(FunctionCall fc, Function f, string name | fc = release and f = fc.getTarget() and - f.hasGlobalName(name) and + f.hasGlobalOrStdName(name) and ( name = "fclose" and resource = fc.getArgument(0) and diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 97.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 97.ql index 501101f706c6..872a7443e6ea 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 97.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 97.ql @@ -22,8 +22,8 @@ predicate containsArray(Type t) { or containsArray(t.getUnderlyingType()) and not exists(TypedefType allowed | allowed = t | - allowed.hasGlobalName("jmp_buf") or - allowed.hasGlobalName("va_list") + allowed.hasGlobalOrStdName("jmp_buf") or + allowed.hasGlobalOrStdName("va_list") ) } diff --git a/cpp/ql/src/semmle/code/cpp/Function.qll b/cpp/ql/src/semmle/code/cpp/Function.qll index 94d11d63575c..8c86d6b12671 100644 --- a/cpp/ql/src/semmle/code/cpp/Function.qll +++ b/cpp/ql/src/semmle/code/cpp/Function.qll @@ -434,7 +434,7 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function { // ... and likewise for destructors. this.(Destructor).getADestruction().mayBeGloballyImpure() else - not exists(string name | this.hasGlobalName(name) | + not exists(string name | this.hasGlobalOrStdName(name) | // Unless it's a function that we know is side-effect-free, it may // have side-effects. name = "strcmp" or diff --git a/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll b/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll index e90aea377818..618afd624c47 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll @@ -5,13 +5,17 @@ import cpp */ predicate allocationFunction(Function f) { exists(string name | - f.hasGlobalName(name) and + f.hasGlobalOrStdName(name) and ( name = "malloc" or name = "calloc" or name = "realloc" or name = "strdup" or - name = "wcsdup" or + name = "wcsdup" + ) + or + f.hasGlobalName(name) and + ( name = "_strdup" or name = "_wcsdup" or name = "_mbsdup" or @@ -59,7 +63,7 @@ predicate allocationCall(FunctionCall fc) { allocationFunction(fc.getTarget()) and ( // realloc(ptr, 0) only frees the pointer - fc.getTarget().hasGlobalName("realloc") implies not fc.getArgument(1).getValue() = "0" + fc.getTarget().hasGlobalOrStdName("realloc") implies not fc.getArgument(1).getValue() = "0" ) } @@ -73,7 +77,10 @@ predicate freeFunction(Function f, int argNum) { name = "free" and argNum = 0 or name = "realloc" and argNum = 0 - or + ) + or + f.hasGlobalOrStdName(name) and + ( name = "ExFreePoolWithTag" and argNum = 0 or name = "ExFreeToLookasideListEx" and argNum = 1 @@ -188,3 +195,4 @@ predicate isDeallocationExpr(Expr e) { e instanceof DeleteExpr or e instanceof DeleteArrayExpr } + diff --git a/cpp/ql/src/semmle/code/cpp/commons/Environment.qll b/cpp/ql/src/semmle/code/cpp/commons/Environment.qll index e36244f4d5c7..f3f1759dd5c0 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Environment.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Environment.qll @@ -28,7 +28,7 @@ class EnvironmentRead extends Expr { private predicate readsEnvironment(Expr read, string sourceDescription) { exists(FunctionCall call, string name | read = call and - call.getTarget().hasGlobalName(name) and + call.getTarget().hasGlobalOrStdName(name) and (name = "getenv" or name = "secure_getenv" or name = "_wgetenv") and sourceDescription = name ) diff --git a/cpp/ql/src/semmle/code/cpp/commons/File.qll b/cpp/ql/src/semmle/code/cpp/commons/File.qll index 192918d25b34..05b4cfeaac49 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/File.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/File.qll @@ -5,8 +5,8 @@ import cpp */ predicate fopenCall(FunctionCall fc) { exists(Function f | f = fc.getTarget() | - f.hasGlobalName("fopen") or - f.hasGlobalName("open") or + f.hasGlobalOrStdName("fopen") or + f.hasGlobalOrStdName("open") or f.hasGlobalName("_open") or f.hasGlobalName("_wopen") or f.hasGlobalName("CreateFile") or @@ -23,16 +23,16 @@ predicate fopenCall(FunctionCall fc) { */ predicate fcloseCall(FunctionCall fc, Expr closed) { exists(Function f | f = fc.getTarget() | - f.hasGlobalName("fclose") and + f.hasGlobalOrStdName("fclose") and closed = fc.getArgument(0) or - f.hasGlobalName("close") and + f.hasGlobalOrStdName("close") and closed = fc.getArgument(0) or - f.hasGlobalName("_close") and + f.hasGlobalOrStdName("_close") and closed = fc.getArgument(0) or - f.hasGlobalName("CloseHandle") and + f.hasGlobalOrStdName("CloseHandle") and closed = fc.getArgument(0) ) } diff --git a/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll b/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll index e10e52df07d5..772bb42e3192 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll @@ -53,8 +53,8 @@ class AnalysedString extends Expr { */ class StrlenCall extends FunctionCall { StrlenCall() { - this.getTarget().hasGlobalName("strlen") or - this.getTarget().hasGlobalName("wcslen") or + this.getTarget().hasGlobalOrStdName("strlen") or + this.getTarget().hasGlobalOrStdName("wcslen") or this.getTarget().hasGlobalName("_mbslen") or this.getTarget().hasGlobalName("_mbslen_l") or this.getTarget().hasGlobalName("_mbstrlen") or diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll b/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll index cfd3efa91f33..69c5963af302 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll @@ -6,7 +6,7 @@ import Nullness */ predicate callDereferences(FunctionCall fc, int i) { exists(string name | - fc.getTarget().hasGlobalName(name) and + fc.getTarget().hasGlobalOrStdName(name) and ( name = "bcopy" and i in [0 .. 1] or diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll b/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll index 54b14aadd9de..caaa5b54e8c6 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll @@ -264,9 +264,9 @@ predicate callMayReturnNull(Call call) { * Holds if `f` may, directly or indirectly, return a null literal. */ predicate mayReturnNull(Function f) { - f.hasGlobalName("malloc") + f.hasGlobalOrStdName("malloc") or - f.hasGlobalName("calloc") + f.hasGlobalOrStdName("calloc") or // f.hasGlobalName("strchr") // or diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll index 5654c1a9c997..58721897802f 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll @@ -7,9 +7,9 @@ class Printf extends FormattingFunction { Printf() { this instanceof TopLevelFunction and ( - hasGlobalName("printf") or + hasGlobalOrStdName("printf") or hasGlobalName("printf_s") or - hasGlobalName("wprintf") or + hasGlobalOrStdName("wprintf") or hasGlobalName("wprintf_s") or hasGlobalName("g_printf") ) and @@ -19,7 +19,7 @@ class Printf extends FormattingFunction { override int getFormatParameterIndex() { result = 0 } override predicate isWideCharDefault() { - hasGlobalName("wprintf") or + hasGlobalOrStdName("wprintf") or hasGlobalName("wprintf_s") } } @@ -31,8 +31,8 @@ class Fprintf extends FormattingFunction { Fprintf() { this instanceof TopLevelFunction and ( - hasGlobalName("fprintf") or - hasGlobalName("fwprintf") or + hasGlobalOrStdName("fprintf") or + hasGlobalOrStdName("fwprintf") or hasGlobalName("g_fprintf") ) and not exists(getDefinition().getFile().getRelativePath()) @@ -40,7 +40,7 @@ class Fprintf extends FormattingFunction { override int getFormatParameterIndex() { result = 1 } - override predicate isWideCharDefault() { hasGlobalName("fwprintf") } + override predicate isWideCharDefault() { hasGlobalOrStdName("fwprintf") } override int getOutputParameterIndex() { result = 0 } } @@ -52,10 +52,10 @@ class Sprintf extends FormattingFunction { Sprintf() { this instanceof TopLevelFunction and ( - hasGlobalName("sprintf") or + hasGlobalOrStdName("sprintf") or hasGlobalName("_sprintf_l") or hasGlobalName("__swprintf_l") or - hasGlobalName("wsprintf") or + hasGlobalOrStdName("wsprintf") or hasGlobalName("g_strdup_printf") or hasGlobalName("g_sprintf") or hasGlobalName("__builtin___sprintf_chk") @@ -99,8 +99,8 @@ class Snprintf extends FormattingFunction { Snprintf() { this instanceof TopLevelFunction and ( - hasGlobalName("snprintf") or // C99 defines snprintf - hasGlobalName("swprintf") or // The s version of wide-char printf is also always the n version + hasGlobalOrStdName("snprintf") or // C99 defines snprintf + hasGlobalOrStdName("swprintf") or // The s version of wide-char printf is also always the n version // Microsoft has _snprintf as well as several other variations hasGlobalName("sprintf_s") or hasGlobalName("snprintf_s") or @@ -160,7 +160,7 @@ class Snprintf extends FormattingFunction { */ predicate returnsFullFormatLength() { ( - hasGlobalName("snprintf") or + hasGlobalOrStdName("snprintf") or hasGlobalName("g_snprintf") or hasGlobalName("__builtin___snprintf_chk") or hasGlobalName("snprintf_s") diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll index c5729c5d0f65..59b8c25700f9 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll @@ -6,7 +6,7 @@ import semmle.code.cpp.models.interfaces.SideEffect class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction { PureStrFunction() { exists(string name | - hasGlobalName(name) and + hasGlobalOrStdName(name) and ( name = "atof" or name = "atoi" or @@ -75,7 +75,7 @@ class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideE class PureFunction extends TaintFunction, SideEffectFunction { PureFunction() { exists(string name | - hasGlobalName(name) and + hasGlobalOrStdName(name) and ( name = "abs" or name = "labs" diff --git a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll index 9988e20290c7..c7bea7eede7d 100644 --- a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll +++ b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll @@ -8,7 +8,7 @@ import semmle.code.cpp.security.FunctionWithWrappers */ class SystemFunction extends FunctionWithWrappers { SystemFunction() { - hasGlobalName("system") or + hasGlobalOrStdName("system") or hasGlobalName("popen") or // Windows variants hasGlobalName("_popen") or diff --git a/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll b/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll index 6363e3a5d143..219f3d0a75b7 100644 --- a/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll +++ b/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll @@ -125,7 +125,7 @@ private predicate fileWrite(Call write, Expr source, Expr dest) { exists(Function f, int s, int d | f = write.getTarget() and source = write.getArgument(s) and dest = write.getArgument(d) | - exists(string name | f.hasGlobalName(name) | + exists(string name | f.hasGlobalOrStdName(name) | // named functions name = "fwrite" and s = 0 and d = 3 or diff --git a/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll b/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll index 751a7a71ec7a..06abfdb454da 100644 --- a/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll +++ b/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll @@ -63,8 +63,8 @@ private predicate outputWrite(Expr write, Expr source) { or // puts, putchar ( - f.hasGlobalName("puts") or - f.hasGlobalName("putchar") + f.hasGlobalOrStdName("puts") or + f.hasGlobalOrStdName("putchar") ) and arg = 0 or diff --git a/cpp/ql/src/semmle/code/cpp/security/Security.qll b/cpp/ql/src/semmle/code/cpp/security/Security.qll index 1a1c1d61ce82..c2b967071a69 100644 --- a/cpp/ql/src/semmle/code/cpp/security/Security.qll +++ b/cpp/ql/src/semmle/code/cpp/security/Security.qll @@ -70,7 +70,7 @@ class SecurityOptions extends string { */ predicate userInputArgument(FunctionCall functionCall, int arg) { exists(string fname | - functionCall.getTarget().hasGlobalName(fname) and + functionCall.getTarget().hasGlobalOrStdName(fname) and exists(functionCall.getArgument(arg)) and ( fname = "read" and arg = 1 @@ -83,6 +83,14 @@ class SecurityOptions extends string { or fname = "gets" and arg = 0 or + fname = "scanf" and arg >= 1 + or + fname = "fscanf" and arg >= 2 + ) + or + functionCall.getTarget().hasGlobalName(fname) and + exists(functionCall.getArgument(arg)) and + ( fname = "getaddrinfo" and arg = 3 or fname = "recv" and arg = 1 @@ -91,10 +99,6 @@ class SecurityOptions extends string { (arg = 1 or arg = 4 or arg = 5) or fname = "recvmsg" and arg = 1 - or - fname = "scanf" and arg >= 1 - or - fname = "fscanf" and arg >= 2 ) ) } diff --git a/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll index 1ef0a6f7092d..b7bdd1416c7c 100644 --- a/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll @@ -425,35 +425,35 @@ private int maxArgIndex(Function f) { /** Functions that copy the value of one argument to another */ private predicate copyValueBetweenArguments(Function f, int sourceArg, int destArg) { - f.hasGlobalName("memcpy") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("memcpy") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("__builtin___memcpy_chk") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("memmove") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("memmove") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("strcat") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("strcat") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("_mbscat") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("wcsncat") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("wcscat") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("strncat") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("strncat") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("_mbsncat") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("wcsncat") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("strcpy") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("strcpy") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("_mbscpy") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("wcscpy") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("wcscpy") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("strncpy") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("strncpy") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("_mbsncpy") and sourceArg = 1 and destArg = 0 or - f.hasGlobalName("wcsncpy") and sourceArg = 1 and destArg = 0 + f.hasGlobalOrStdName("wcsncpy") and sourceArg = 1 and destArg = 0 or f.hasGlobalName("inet_aton") and sourceArg = 0 and destArg = 1 or @@ -473,31 +473,31 @@ private predicate returnArgument(Function f, int sourceArg) { or f.hasGlobalName("__builtin___memcpy_chk") and sourceArg = 0 or - f.hasGlobalName("memmove") and sourceArg = 0 + f.hasGlobalOrStdName("memmove") and sourceArg = 0 or - f.hasGlobalName("strcat") and sourceArg = 0 + f.hasGlobalOrStdName("strcat") and sourceArg = 0 or f.hasGlobalName("_mbscat") and sourceArg = 0 or - f.hasGlobalName("wcsncat") and sourceArg = 0 + f.hasGlobalOrStdName("wcsncat") and sourceArg = 0 or - f.hasGlobalName("strncat") and sourceArg = 0 + f.hasGlobalOrStdName("strncat") and sourceArg = 0 or f.hasGlobalName("_mbsncat") and sourceArg = 0 or - f.hasGlobalName("wcsncat") and sourceArg = 0 + f.hasGlobalOrStdName("wcsncat") and sourceArg = 0 or - f.hasGlobalName("strcpy") and sourceArg = 0 + f.hasGlobalOrStdName("strcpy") and sourceArg = 0 or f.hasGlobalName("_mbscpy") and sourceArg = 0 or - f.hasGlobalName("wcscpy") and sourceArg = 0 + f.hasGlobalOrStdName("wcscpy") and sourceArg = 0 or - f.hasGlobalName("strncpy") and sourceArg = 0 + f.hasGlobalOrStdName("strncpy") and sourceArg = 0 or f.hasGlobalName("_mbsncpy") and sourceArg = 0 or - f.hasGlobalName("wcsncpy") and sourceArg = 0 + f.hasGlobalOrStdName("wcsncpy") and sourceArg = 0 or f.hasGlobalName("inet_ntoa") and sourceArg = 0 or From 68c38ba34af4d29b86379bb1aea1109230c7dc47 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Fri, 12 Jul 2019 10:34:15 -0700 Subject: [PATCH 4/8] C++: Add change note --- change-notes/1.23/analysis-cpp.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/change-notes/1.23/analysis-cpp.md b/change-notes/1.23/analysis-cpp.md index 5a97e1347bb3..f0f0889c2823 100644 --- a/change-notes/1.23/analysis-cpp.md +++ b/change-notes/1.23/analysis-cpp.md @@ -42,3 +42,5 @@ The following changes in version 1.23 affect C/C++ analysis in all applications. clarity (e.g. `isOutReturnPointer()` to `isReturnValueDeref()`). The existing member predicates have been deprecated, and will be removed in a future release. Code that uses the old member predicates should be updated to use the corresponding new member predicate. +* The predicates `Declaration.hasStdName()` and `Declaration.hasGlobalOrStdName` + have been added, simplifying handling of C++ standard library functions. From 4018ed67a60168975d311560274b0225b51630ec Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Wed, 2 Oct 2019 11:21:10 -0700 Subject: [PATCH 5/8] C++: respond to PR comments --- cpp/ql/src/Critical/OverflowCalculated.ql | 2 +- cpp/ql/src/Critical/OverflowStatic.ql | 2 +- cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql | 5 +---- cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql | 2 +- cpp/ql/src/semmle/code/cpp/commons/File.qll | 4 ++-- cpp/ql/src/semmle/code/cpp/security/Security.qll | 4 ++-- cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll | 2 +- 7 files changed, 9 insertions(+), 12 deletions(-) diff --git a/cpp/ql/src/Critical/OverflowCalculated.ql b/cpp/ql/src/Critical/OverflowCalculated.ql index c528be8ca242..0ad898decd5f 100644 --- a/cpp/ql/src/Critical/OverflowCalculated.ql +++ b/cpp/ql/src/Critical/OverflowCalculated.ql @@ -36,7 +36,7 @@ predicate spaceProblem(FunctionCall append, string msg) { buffer.getAnAccess() = strlen.getStringExpr() and ( insert.getTarget().hasGlobalOrStdName("strcpy") or - insert.getTarget().hasGlobalName("strncpy") + insert.getTarget().hasGlobalOrStdName("strncpy") ) and ( append.getTarget().hasGlobalOrStdName("strcat") or diff --git a/cpp/ql/src/Critical/OverflowStatic.ql b/cpp/ql/src/Critical/OverflowStatic.ql index 8cee25007972..82ffc879331e 100644 --- a/cpp/ql/src/Critical/OverflowStatic.ql +++ b/cpp/ql/src/Critical/OverflowStatic.ql @@ -58,7 +58,7 @@ predicate overflowOffsetInLoop(BufferAccess bufaccess, string msg) { } predicate bufferAndSizeFunction(Function f, int buf, int size) { - f.hasGlobalOrStdName("read") and buf = 1 and size = 2 + f.hasGlobalName("read") and buf = 1 and size = 2 or f.hasGlobalOrStdName("fgets") and buf = 0 and size = 1 or diff --git a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql index fba0ddda4a13..5d02533124a0 100644 --- a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -34,10 +34,7 @@ class FileFunction extends FunctionWithWrappers { nme.matches("CreateFile%") ) or - exists(string nme | this.hasStdName(nme) | - nme = "fopen" or - nme = "open" - ) + this.hasStdName("fopen") or // on any of the fstream classes, or filebuf exists(string nme | this.getDeclaringType().hasStdName(nme) | diff --git a/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql b/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql index 77225ad05776..473f0f72f122 100644 --- a/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql +++ b/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql @@ -66,7 +66,7 @@ class VarargsFunction extends Function { } predicate isWhitelisted() { - this.hasGlobalOrStdName("open") or + this.hasGlobalName("open") or this.hasGlobalName("fcntl") or this.hasGlobalName("ptrace") } diff --git a/cpp/ql/src/semmle/code/cpp/commons/File.qll b/cpp/ql/src/semmle/code/cpp/commons/File.qll index 05b4cfeaac49..ea8f8b9dc654 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/File.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/File.qll @@ -26,10 +26,10 @@ predicate fcloseCall(FunctionCall fc, Expr closed) { f.hasGlobalOrStdName("fclose") and closed = fc.getArgument(0) or - f.hasGlobalOrStdName("close") and + f.hasGlobalName("close") and closed = fc.getArgument(0) or - f.hasGlobalOrStdName("_close") and + f.hasGlobalName("_close") and closed = fc.getArgument(0) or f.hasGlobalOrStdName("CloseHandle") and diff --git a/cpp/ql/src/semmle/code/cpp/security/Security.qll b/cpp/ql/src/semmle/code/cpp/security/Security.qll index c2b967071a69..c12a70b52c6a 100644 --- a/cpp/ql/src/semmle/code/cpp/security/Security.qll +++ b/cpp/ql/src/semmle/code/cpp/security/Security.qll @@ -73,8 +73,6 @@ class SecurityOptions extends string { functionCall.getTarget().hasGlobalOrStdName(fname) and exists(functionCall.getArgument(arg)) and ( - fname = "read" and arg = 1 - or fname = "fread" and arg = 0 or fname = "fgets" and arg = 0 @@ -91,6 +89,8 @@ class SecurityOptions extends string { functionCall.getTarget().hasGlobalName(fname) and exists(functionCall.getArgument(arg)) and ( + fname = "read" and arg = 1 + or fname = "getaddrinfo" and arg = 3 or fname = "recv" and arg = 1 diff --git a/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll index b7bdd1416c7c..31d8ad00f9cd 100644 --- a/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll @@ -459,7 +459,7 @@ private predicate copyValueBetweenArguments(Function f, int sourceArg, int destA or f.hasGlobalName("inet_pton") and sourceArg = 1 and destArg = 2 or - f.hasGlobalName("strftime") and sourceArg in [2 .. maxArgIndex(f)] and destArg = 0 + f.hasGlobalOrStdName("strftime") and sourceArg in [2 .. maxArgIndex(f)] and destArg = 0 or exists(FormattingFunction ff | ff = f | sourceArg in [ff.getFormatParameterIndex() .. maxArgIndex(f)] and From 5c084f8b39c9503c1326ef68befeb325e435a023 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Mon, 7 Oct 2019 14:16:21 -0700 Subject: [PATCH 6/8] C++: respond to more PR comments --- cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql | 4 ++-- cpp/ql/src/semmle/code/cpp/Declaration.qll | 8 +------- cpp/ql/src/semmle/code/cpp/commons/File.qll | 2 +- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql index 5d02533124a0..42a29b96268b 100644 --- a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -34,10 +34,10 @@ class FileFunction extends FunctionWithWrappers { nme.matches("CreateFile%") ) or - this.hasStdName("fopen") + this.hasQualifiedName("std", "fopen") or // on any of the fstream classes, or filebuf - exists(string nme | this.getDeclaringType().hasStdName(nme) | + exists(string nme | this.getDeclaringType().hasQualifiedName("std", nme) | nme = "basic_fstream" or nme = "basic_ifstream" or nme = "basic_ofstream" or diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 7c38acc564ee..88ee21da5580 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -119,17 +119,11 @@ abstract class Declaration extends Locatable, @declaration { /** Holds if this declaration has the given name in the global namespace. */ predicate hasGlobalName(string name) { this.hasQualifiedName("", "", name) } - - /** Holds if this declaration has the given name in the `std` namespace. */ - predicate hasStdName(string name) { - this.hasQualifiedName("std", "", name) - } - /** Holds if this declaration has the given name in the global namespace or the `std` namespace. */ predicate hasGlobalOrStdName(string name) { this.hasGlobalName(name) or - this.hasStdName(name) + this.hasQualifiedName("std", "", name) } /** Gets a specifier of this declaration. */ diff --git a/cpp/ql/src/semmle/code/cpp/commons/File.qll b/cpp/ql/src/semmle/code/cpp/commons/File.qll index ea8f8b9dc654..5808d704e385 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/File.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/File.qll @@ -6,7 +6,7 @@ import cpp predicate fopenCall(FunctionCall fc) { exists(Function f | f = fc.getTarget() | f.hasGlobalOrStdName("fopen") or - f.hasGlobalOrStdName("open") or + f.hasGlobalName("open") or f.hasGlobalName("_open") or f.hasGlobalName("_wopen") or f.hasGlobalName("CreateFile") or From 7fa367d6cf9342b1065eb1250e83c3e454232bbb Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Mon, 7 Oct 2019 16:37:35 -0700 Subject: [PATCH 7/8] C++: autoformat --- cpp/ql/src/semmle/code/cpp/commons/Alloc.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll b/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll index 618afd624c47..8462cb13b1d3 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll @@ -195,4 +195,3 @@ predicate isDeallocationExpr(Expr e) { e instanceof DeleteExpr or e instanceof DeleteArrayExpr } - From 9554513cd6de7feeeb55c51f8a81c3627ed13c7d Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Tue, 12 Nov 2019 10:16:01 -0800 Subject: [PATCH 8/8] autoformat --- cpp/ql/src/Critical/OverflowCalculated.ql | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/ql/src/Critical/OverflowCalculated.ql b/cpp/ql/src/Critical/OverflowCalculated.ql index 0ad898decd5f..fbca4a20a8f0 100644 --- a/cpp/ql/src/Critical/OverflowCalculated.ql +++ b/cpp/ql/src/Critical/OverflowCalculated.ql @@ -13,9 +13,7 @@ import cpp class MallocCall extends FunctionCall { - MallocCall() { - this.getTarget().hasGlobalOrStdName("malloc") - } + MallocCall() { this.getTarget().hasGlobalOrStdName("malloc") } Expr getAllocatedSize() { if this.getArgument(0) instanceof VariableAccess