From 4459c8e7c68da399c3eb402961100b483d7ed2af Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 20 Dec 2021 17:53:09 +0100 Subject: [PATCH 1/3] run the redundant-cast patch --- ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll | 2 +- ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll | 6 +++--- ql/src/experimental/CWE-400/DatabaseCallInLoop.ql | 4 ++-- ql/src/experimental/IntegerOverflow/RangeAnalysis.qll | 3 +-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll index 35caa5d7cbc..3b9d1571343 100644 --- a/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -652,7 +652,7 @@ module CFG { AtomicTree() { exists(Expr e | - e = this.(Expr) and + e = this and e.isConst() and nd = mkExprOrSkipNode(this) | diff --git a/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll index 8f2eee86ec6..f33048ed08a 100644 --- a/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll @@ -289,7 +289,7 @@ private predicate isPossibleInputNode(DataFlow::Node inputNode, FuncDef fd) { private ControlFlow::Node getANonTestPassingPredecessor( ControlFlow::Node succ, DataFlow::Node inputNode ) { - isPossibleInputNode(inputNode, succ.getRoot().(FuncDef)) and + isPossibleInputNode(inputNode, succ.getRoot()) and result = succ.getAPredecessor() and not exists(Expr testExpr, DataFlow::Node switchExprNode | flowsToSwitchExpression(inputNode, switchExprNode) and @@ -301,7 +301,7 @@ private ControlFlow::Node getANonTestPassingPredecessor( private ControlFlow::Node getANonTestPassingReachingNodeRecursive( ControlFlow::Node n, DataFlow::Node inputNode ) { - isPossibleInputNode(inputNode, n.getRoot().(FuncDef)) and + isPossibleInputNode(inputNode, n.getRoot()) and ( result = n or result = @@ -328,7 +328,7 @@ private ControlFlow::Node getANonTestPassingReachingNodeBase( private predicate mustPassConstantCaseTestToReach( IR::ReturnInstruction ret, DataFlow::Node inputNode ) { - isPossibleInputNode(inputNode, ret.getRoot().(FuncDef)) and + isPossibleInputNode(inputNode, ret.getRoot()) and not exists(ControlFlow::Node entry | entry = ret.getRoot().getEntryNode() | entry = getANonTestPassingReachingNodeBase(ret, inputNode) ) diff --git a/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql b/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql index 2e4f25fe495..253d598835d 100644 --- a/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql +++ b/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql @@ -47,10 +47,10 @@ predicate callGraphEdge(CallGraphNode pred, CallGraphNode succ) { pred.(CallExpr) = succ.(FuncDef).getACall().asExpr() or // Go from a function to an enclosed loop. - pred.(FuncDef) = succ.(LoopStmt).getEnclosingFunction() + pred = succ.(LoopStmt).getEnclosingFunction() or // Go from a function to an enclosed call. - pred.(FuncDef) = succ.(CallExpr).getEnclosingFunction() + pred = succ.(CallExpr).getEnclosingFunction() } query predicate edges(CallGraphNode pred, CallGraphNode succ) { diff --git a/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll b/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll index a1d2ec581e8..cff3873a0d6 100644 --- a/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll +++ b/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll @@ -535,8 +535,7 @@ float getAnSsaLowerBound(SsaDefinition def) { predicate ssaDependsOnSsa(SsaDefinition nextDef, SsaDefinition prevDef) { //SSA definition coresponding to a `SimpleAssignStmt` exists(SimpleAssignStmt simpleAssign, int i | - nextDef.(SsaExplicitDefinition).getInstruction().(IR::AssignInstruction) = - IR::assignInstruction(simpleAssign, i) and + nextDef.(SsaExplicitDefinition).getInstruction() = IR::assignInstruction(simpleAssign, i) and ssaDependsOnExpr(prevDef, simpleAssign.getRhs()) ) or From d339f136298d2caab87f42068cf8a7b87a46eb2a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 20 Dec 2021 17:54:18 +0100 Subject: [PATCH 2/3] run the non-us-language patch --- ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll | 2 +- ql/lib/semmle/go/dataflow/SsaImpl.qll | 2 +- ql/lib/semmle/go/frameworks/Encoding.qll | 2 +- ql/lib/semmle/go/frameworks/Protobuf.qll | 2 +- ql/lib/semmle/go/frameworks/Revel.qll | 2 +- ql/lib/semmle/go/frameworks/Testing.qll | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll index 3b9d1571343..743b8e4fae6 100644 --- a/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -489,7 +489,7 @@ module CFG { /** * A completion indicating that an expression was successfully evaluated to Boolean value `b`. * - * Note that many Boolean expressions are modelled as having completion `Done()` instead. + * Note that many Boolean expressions are modeled as having completion `Done()` instead. * Completion `Bool` is only used in contexts where the Boolean value can be determined. */ Bool(boolean b) { b = true or b = false } or diff --git a/ql/lib/semmle/go/dataflow/SsaImpl.qll b/ql/lib/semmle/go/dataflow/SsaImpl.qll index a15e8595b99..0db37ac03ce 100644 --- a/ql/lib/semmle/go/dataflow/SsaImpl.qll +++ b/ql/lib/semmle/go/dataflow/SsaImpl.qll @@ -100,7 +100,7 @@ private module Internal { /** * Holds if the `i`th node of basic block `bb` may induce a pseudo-definition for - * modelling updates to captured variable `v`. Whether the definition is actually + * modeling updates to captured variable `v`. Whether the definition is actually * introduced depends on whether `v` is live at this point in the program. */ private predicate mayCapture(ReachableBasicBlock bb, int i, SsaSourceVariable v) { diff --git a/ql/lib/semmle/go/frameworks/Encoding.qll b/ql/lib/semmle/go/frameworks/Encoding.qll index ea3d678305b..34af4ce6ed7 100644 --- a/ql/lib/semmle/go/frameworks/Encoding.qll +++ b/ql/lib/semmle/go/frameworks/Encoding.qll @@ -1,5 +1,5 @@ /** - * Provides classes modelling taint propagation through marshalling and encoding functions. + * Provides classes modeling taint propagation through marshalling and encoding functions. */ import go diff --git a/ql/lib/semmle/go/frameworks/Protobuf.qll b/ql/lib/semmle/go/frameworks/Protobuf.qll index cdaf52990e8..66b3bba8de9 100644 --- a/ql/lib/semmle/go/frameworks/Protobuf.qll +++ b/ql/lib/semmle/go/frameworks/Protobuf.qll @@ -60,7 +60,7 @@ module Protobuf { } /** - * Additional taint-flow step modelling flow from `MarshalInput.Message` to `MarshalOutput`, + * Additional taint-flow step modeling flow from `MarshalInput.Message` to `MarshalOutput`, * mediated by a `MarshalOptions.MarshalState` call. * * Note we can taint the whole `MarshalOutput` as it only has one field (`Buf`), and taint- diff --git a/ql/lib/semmle/go/frameworks/Revel.qll b/ql/lib/semmle/go/frameworks/Revel.qll index 62f038d9120..1ccad431b84 100644 --- a/ql/lib/semmle/go/frameworks/Revel.qll +++ b/ql/lib/semmle/go/frameworks/Revel.qll @@ -5,7 +5,7 @@ import go private import semmle.go.security.OpenUrlRedirectCustomizations -/** Provides classes and methods modelling the Revel web framework. */ +/** Provides classes and methods modeling the Revel web framework. */ module Revel { /** Gets the package name `github.com/revel/revel`. */ string packagePath() { result = package(["github.com/revel", "github.com/robfig"], "revel") } diff --git a/ql/lib/semmle/go/frameworks/Testing.qll b/ql/lib/semmle/go/frameworks/Testing.qll index 728e33f31ef..2df3d6c0ddd 100644 --- a/ql/lib/semmle/go/frameworks/Testing.qll +++ b/ql/lib/semmle/go/frameworks/Testing.qll @@ -87,7 +87,7 @@ module TestFile { } } -/** Provides classes modelling Ginkgo. */ +/** Provides classes modeling Ginkgo. */ module Ginkgo { /** Gets the package path `github.com/onsi/ginkgo`. */ string packagePath() { result = package("github.com/onsi/ginkgo", "") } From afe7ee17a04848773613e8655e76c5598b6783bd Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 20 Dec 2021 17:55:19 +0100 Subject: [PATCH 3/3] run the use-set-literals patch --- ql/lib/semmle/go/concepts/GeneratedFile.qll | 16 ++-- ql/lib/semmle/go/frameworks/SQL.qll | 10 +-- ql/lib/semmle/go/frameworks/Stdlib.qll | 6 +- .../go/frameworks/SystemCommandExecutors.qll | 87 ++++--------------- .../semmle/go/frameworks/stdlib/IoIoutil.qll | 6 +- .../semmle/go/frameworks/stdlib/NetHttp.qll | 10 +-- .../experimental/CWE-327/CryptoLibraries.qll | 61 ++++--------- .../SensitiveActions/DummyPasswords.ql | 14 ++- 8 files changed, 49 insertions(+), 161 deletions(-) diff --git a/ql/lib/semmle/go/concepts/GeneratedFile.qll b/ql/lib/semmle/go/concepts/GeneratedFile.qll index c70d3104282..dec6872120c 100644 --- a/ql/lib/semmle/go/concepts/GeneratedFile.qll +++ b/ql/lib/semmle/go/concepts/GeneratedFile.qll @@ -13,15 +13,15 @@ module GeneratedFile { abstract class Range extends File { } private string generatorCommentRegex() { - result = "Generated By\\b.*\\bDo not edit" or result = - "This (file|class|interface|art[ei]fact) (was|is|(has been)) (?:auto[ -]?)?gener(e?)ated" or - result = "Any modifications to this file will be lost" or - result = - "This (file|class|interface|art[ei]fact) (was|is) (?:mechanically|automatically) generated" or - result = "The following code was (?:auto[ -]?)?generated (?:by|from)" or - result = "Autogenerated by Thrift" or - result = "(Code g|G)enerated from .* by ANTLR" + [ + "Generated By\\b.*\\bDo not edit", + "This (file|class|interface|art[ei]fact) (was|is|(has been)) (?:auto[ -]?)?gener(e?)ated", + "Any modifications to this file will be lost", + "This (file|class|interface|art[ei]fact) (was|is) (?:mechanically|automatically) generated", + "The following code was (?:auto[ -]?)?generated (?:by|from)", "Autogenerated by Thrift", + "(Code g|G)enerated from .* by ANTLR" + ] } private class CommentHeuristicGeneratedFile extends Range { diff --git a/ql/lib/semmle/go/frameworks/SQL.qll b/ql/lib/semmle/go/frameworks/SQL.qll index 9f01d4c7b91..acf8be77d7f 100644 --- a/ql/lib/semmle/go/frameworks/SQL.qll +++ b/ql/lib/semmle/go/frameworks/SQL.qll @@ -143,15 +143,7 @@ module SQL { or exists(string tp, string m | f.(Method).hasQualifiedName(gopgorm(), tp, m) | tp = "Query" and - ( - m = "ColumnExpr" or - m = "For" or - m = "Having" or - m = "Where" or - m = "WhereIn" or - m = "WhereInMulti" or - m = "WhereOr" - ) and + m = ["ColumnExpr", "For", "Having", "Where", "WhereIn", "WhereInMulti", "WhereOr"] and arg = 0 or tp = "Query" and diff --git a/ql/lib/semmle/go/frameworks/Stdlib.qll b/ql/lib/semmle/go/frameworks/Stdlib.qll index 063214fb570..c351ce23335 100644 --- a/ql/lib/semmle/go/frameworks/Stdlib.qll +++ b/ql/lib/semmle/go/frameworks/Stdlib.qll @@ -174,11 +174,7 @@ module URL { class UrlGetter extends TaintTracking::FunctionModel, Method { UrlGetter() { exists(string m | hasQualifiedName("net/url", "URL", m) | - m = "EscapedPath" or - m = "Hostname" or - m = "Port" or - m = "Query" or - m = "RequestURI" + m = ["EscapedPath", "Hostname", "Port", "Query", "RequestURI"] ) } diff --git a/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll b/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll index 4d114aedd07..5b0f8cb2022 100644 --- a/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll +++ b/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll @@ -126,47 +126,14 @@ private class ShellLike extends DataFlow::Node { } private string getASudoCommand() { - result = "sudo" or - result = "sudo_root" or - result = "su" or - result = "sudoedit" or - result = "doas" or - result = "access" or - result = "vsys" or - result = "userv" or - result = "sus" or - result = "super" or - result = "priv" or - result = "calife" or - result = "ssu" or - result = "su1" or - result = "op" or - result = "sudowin" or - result = "sudown" or - result = "chroot" or - result = "fakeroot" or - result = "fakeroot-sysv" or - result = "fakeroot-tcp" or - result = "fstab-decode" or - result = "jrunscript" or - result = "nohup" or - result = "parallel" or - result = "find" or - result = "pkexec" or - result = "sg" or - result = "sem" or - result = "runcon" or - result = "runuser" or - result = "stdbuf" or - result = "system" or - result = "timeout" or - result = "xargs" or - result = "time" or - result = "awk" or - result = "gawk" or - result = "mawk" or - result = "nawk" or - result = "git" + result = + [ + "sudo", "sudo_root", "priv", "calife", "ssu", "su1", "op", "sudowin", "sudown", "chroot", + "fakeroot", "fakeroot-sysv", "su", "fakeroot-tcp", "fstab-decode", "jrunscript", "nohup", + "parallel", "find", "pkexec", "sg", "sem", "runcon", "sudoedit", "runuser", "stdbuf", + "system", "timeout", "xargs", "time", "awk", "gawk", "mawk", "nawk", "doas", "git", "access", + "vsys", "userv", "sus", "super" + ] } /** @@ -213,31 +180,12 @@ private predicate isSudoOrSimilar(DataFlow::Node node) { } private string getAShellCommand() { - result = "bash" or - result = "sh" or - result = "sh.distrib" or - result = "rbash" or - result = "dash" or - result = "zsh" or - result = "csh" or - result = "tcsh" or - result = "fish" or - result = "pwsh" or - result = "elvish" or - result = "oh" or - result = "ion" or - result = "ksh" or - result = "rksh" or - result = "tksh" or - result = "mksh" or - result = "nu" or - result = "oksh" or - result = "osh" or - result = "shpp" or - result = "xiki" or - result = "xonsh" or - result = "yash" or - result = "env" + result = + [ + "bash", "sh", "elvish", "oh", "ion", "ksh", "rksh", "tksh", "mksh", "nu", "oksh", "osh", + "sh.distrib", "shpp", "xiki", "xonsh", "yash", "env", "rbash", "dash", "zsh", "csh", "tcsh", + "fish", "pwsh" + ] } /** @@ -252,12 +200,7 @@ private predicate isShell(DataFlow::Node node) { } private string getAnInterpreterName() { - result = "python" or - result = "php" or - result = "ruby" or - result = "perl" or - result = "node" or - result = "nodejs" + result = ["python", "php", "ruby", "perl", "node", "nodejs"] } /** diff --git a/ql/lib/semmle/go/frameworks/stdlib/IoIoutil.qll b/ql/lib/semmle/go/frameworks/stdlib/IoIoutil.qll index 70548a64672..4a3de941211 100644 --- a/ql/lib/semmle/go/frameworks/stdlib/IoIoutil.qll +++ b/ql/lib/semmle/go/frameworks/stdlib/IoIoutil.qll @@ -9,11 +9,7 @@ module IoIoutil { private class IoUtilFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode { IoUtilFileSystemAccess() { exists(string fn | getTarget().hasQualifiedName("io/ioutil", fn) | - fn = "ReadDir" or - fn = "ReadFile" or - fn = "TempDir" or - fn = "TempFile" or - fn = "WriteFile" + fn = ["ReadDir", "ReadFile", "TempDir", "TempFile", "WriteFile"] ) } diff --git a/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll b/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll index e95141b1aaa..ae6fcd2a582 100644 --- a/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll +++ b/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll @@ -28,14 +28,8 @@ module NetHttp { DataFlow::FieldReadNode { UserControlledRequestField() { exists(string fieldName | this.getField().hasQualifiedName("net/http", "Request", fieldName) | - fieldName = "Body" or - fieldName = "GetBody" or - fieldName = "Form" or - fieldName = "PostForm" or - fieldName = "MultipartForm" or - fieldName = "Header" or - fieldName = "Trailer" or - fieldName = "URL" + fieldName = + ["Body", "GetBody", "Form", "PostForm", "MultipartForm", "Header", "Trailer", "URL"] ) } } diff --git a/ql/src/experimental/CWE-327/CryptoLibraries.qll b/ql/src/experimental/CWE-327/CryptoLibraries.qll index 5223ce2f24c..084e971fdeb 100644 --- a/ql/src/experimental/CWE-327/CryptoLibraries.qll +++ b/ql/src/experimental/CWE-327/CryptoLibraries.qll @@ -20,60 +20,31 @@ import go */ private module AlgorithmNames { predicate isStrongHashingAlgorithm(string name) { - name = "DSA" or - name = "ED25519" or - name = "ES256" or - name = "ECDSA256" or - name = "ES384" or - name = "ECDSA384" or - name = "ES512" or - name = "ECDSA512" or - name = "SHA2" or - name = "SHA224" or - name = "SHA256" or - name = "SHA384" or - name = "SHA512" or - name = "SHA3" + name = + [ + "DSA", "ED25519", "SHA256", "SHA384", "SHA512", "SHA3", "ES256", "ECDSA256", "ES384", + "ECDSA384", "ES512", "ECDSA512", "SHA2", "SHA224" + ] } predicate isWeakHashingAlgorithm(string name) { - name = "HAVEL128" or - name = "MD2" or - name = "MD4" or - name = "MD5" or - name = "PANAMA" or - name = "RIPEMD" or - name = "RIPEMD128" or - name = "RIPEMD256" or - name = "RIPEMD320" or - name = "SHA0" or - name = "SHA1" + name = + [ + "HAVEL128", "MD2", "SHA1", "MD4", "MD5", "PANAMA", "RIPEMD", "RIPEMD128", "RIPEMD256", + "RIPEMD320", "SHA0" + ] } predicate isStrongEncryptionAlgorithm(string name) { - name = "AES" or - name = "AES128" or - name = "AES192" or - name = "AES256" or - name = "AES512" or - name = "RSA" or - name = "RABBIT" or - name = "BLOWFISH" + name = ["AES", "AES128", "AES192", "AES256", "AES512", "RSA", "RABBIT", "BLOWFISH"] } predicate isWeakEncryptionAlgorithm(string name) { - name = "DES" or - name = "3DES" or - name = "TRIPLEDES" or - name = "TDEA" or - name = "TRIPLEDEA" or - name = "ARC2" or - name = "RC2" or - name = "ARC4" or - name = "RC4" or - name = "ARCFOUR" or - name = "ARC5" or - name = "RC5" + name = + [ + "DES", "3DES", "ARC5", "RC5", "TRIPLEDES", "TDEA", "TRIPLEDEA", "ARC2", "RC2", "ARC4", + "RC4", "ARCFOUR" + ] } predicate isStrongPasswordHashingAlgorithm(string name) { diff --git a/ql/test/library-tests/semmle/go/security/SensitiveActions/DummyPasswords.ql b/ql/test/library-tests/semmle/go/security/SensitiveActions/DummyPasswords.ql index cc881c3b706..e62883ff265 100644 --- a/ql/test/library-tests/semmle/go/security/SensitiveActions/DummyPasswords.ql +++ b/ql/test/library-tests/semmle/go/security/SensitiveActions/DummyPasswords.ql @@ -2,15 +2,11 @@ import go import semmle.go.security.SensitiveActions string getASamplePassword() { - result = "abcdefgh" or - result = "sOKY6ccizpmvF*32so%Q" or - result = "XXXXXXXX" or - result = "example_password" or - result = "change_me" or - result = "" or - result = "insert-auth-from-gui" or - result = "admin" or - result = "root" + result = + [ + "abcdefgh", "sOKY6ccizpmvF*32so%Q", "XXXXXXXX", "example_password", "change_me", "", + "insert-auth-from-gui", "admin", "root" + ] } from string password, boolean isDummy