From 2b6f6338240c4a908df4f9818ec501e553ed41ef Mon Sep 17 00:00:00 2001 From: Claudiu-Vlad Ursache Date: Thu, 25 Mar 2021 12:15:40 +0100 Subject: [PATCH 1/4] Bump CPG version --- project/Versions.scala | 2 +- .../scala/io/joern/scanners/c/CopyLoops.scala | 2 +- .../io/joern/scanners/c/CredentialDrop.scala | 4 ++-- .../io/joern/scanners/c/DangerousFunctions.scala | 14 +++++++------- .../io/joern/scanners/c/HeapBasedOverflow.scala | 2 +- .../io/joern/scanners/c/IntegerTruncations.scala | 16 ++++++++-------- src/main/scala/io/joern/scanners/c/Metrics.scala | 12 ++++++------ .../io/joern/scanners/c/NullTermination.scala | 2 +- .../scala/io/joern/scanners/c/RetvalChecks.scala | 2 +- .../io/joern/scanners/c/SignedLeftShift.scala | 2 +- .../scala/io/joern/scanners/c/UseAfterFree.scala | 6 +++--- .../joern/scanners/java/DangerousFunctions.scala | 2 +- 12 files changed, 33 insertions(+), 33 deletions(-) diff --git a/project/Versions.scala b/project/Versions.scala index 63e1762..55ef54f 100644 --- a/project/Versions.scala +++ b/project/Versions.scala @@ -1,4 +1,4 @@ /* Declare dependency versions in one place */ object Versions { - val cpg = "1.3.99" + val cpg = "1.3.102" } diff --git a/src/main/scala/io/joern/scanners/c/CopyLoops.scala b/src/main/scala/io/joern/scanners/c/CopyLoops.scala index 824479b..b78ab63 100644 --- a/src/main/scala/io/joern/scanners/c/CopyLoops.scala +++ b/src/main/scala/io/joern/scanners/c/CopyLoops.scala @@ -9,7 +9,7 @@ object CopyLoops extends QueryBundle { @q def isCopyLoop(): Query = - queryInit( + Query.make( "copy-loop", Crew.fabs, "Copy loop detected", diff --git a/src/main/scala/io/joern/scanners/c/CredentialDrop.scala b/src/main/scala/io/joern/scanners/c/CredentialDrop.scala index 9d36aad..3098ae0 100644 --- a/src/main/scala/io/joern/scanners/c/CredentialDrop.scala +++ b/src/main/scala/io/joern/scanners/c/CredentialDrop.scala @@ -11,7 +11,7 @@ object CredentialDrop extends QueryBundle { @q def userCredDrop(): Query = - queryInit( + Query.make( "setuid-without-setgid", Crew.malte, "Process user ID is changed without changing groups first", @@ -33,7 +33,7 @@ object CredentialDrop extends QueryBundle { @q def groupCredDrop(): Query = - queryInit( + Query.make( "setgid-without-setgroups", Crew.malte, "Process group membership is changed without setting ancillary groups first", diff --git a/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala b/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala index e3a213d..55b4395 100644 --- a/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala +++ b/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala @@ -11,7 +11,7 @@ object DangerousFunctions extends QueryBundle { @q def getsUsed(): Query = - queryInit( + Query.make( "call-to-gets", Crew.suchakra, "Dangerous function gets() used", @@ -28,7 +28,7 @@ object DangerousFunctions extends QueryBundle { @q def argvUsedInPrintf(): Query = - queryInit( + Query.make( "format-controlled-printf", Crew.suchakra, "Non-constant format string passed to printf/sprintf/vsprintf", @@ -52,7 +52,7 @@ object DangerousFunctions extends QueryBundle { @q def scanfUsed(): Query = - queryInit( + Query.make( "call-to-scanf", Crew.suchakra, "Insecure function scanf() used", @@ -68,7 +68,7 @@ object DangerousFunctions extends QueryBundle { @q def strcatUsed(): Query = - queryInit( + Query.make( "call-to-strcat", Crew.suchakra, "Dangerous functions `strcat` or `strncat` used", @@ -85,7 +85,7 @@ object DangerousFunctions extends QueryBundle { @q def strcpyUsed(): Query = - queryInit( + Query.make( "call-to-strcpy", Crew.suchakra, "Dangerous functions `strcpy` or `strncpy` used", @@ -104,7 +104,7 @@ object DangerousFunctions extends QueryBundle { @q def strtokUsed(): Query = - queryInit( + Query.make( "call-to-strtok", Crew.suchakra, "Dangerous function strtok() used", @@ -122,7 +122,7 @@ object DangerousFunctions extends QueryBundle { @q def getwdUsed(): Query = - queryInit( + Query.make( "call-to-getwd", Crew.claudiu, "Dangerous function getwd() used", diff --git a/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala b/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala index c8b3ff0..063d9ab 100644 --- a/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala +++ b/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala @@ -20,7 +20,7 @@ object HeapBasedOverflow extends QueryBundle { * */ @q def mallocMemcpyIntOverflow()(implicit context: EngineContext): Query = - queryInit( + Query.make( "malloc-memcpy-int-overflow", Crew.fabs, "Dangerous copy-operation into heap-allocated buffer", diff --git a/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala b/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala index c485492..ac1cf34 100644 --- a/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala +++ b/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala @@ -16,24 +16,24 @@ object IntegerTruncations extends QueryBundle { * */ @q def strlenAssignmentTruncations(): Query = - queryInit( - "strlen-truncation", - Crew.fabs, - "Truncation in assignment involving `strlen` call", - """ + Query.make( + name = "strlen-truncation", + author = Crew.fabs, + title = "Truncation in assignment involving `strlen` call", + description = """ |The return value of `strlen` is stored in a variable that is known |to be of type `int` as opposed to `size_t`. `int` is only 32 bit |wide on many 64 bit platforms, and thus, this may result in a |truncation. |""".stripMargin, - 2, { cpg => + score = 2, withStrRep({ cpg => cpg .method("strlen") .callIn .inAssignment .target .evalType("(g?)int") - }, - List(QueryTags.integers) + }), + tags = List(QueryTags.integers) ) } diff --git a/src/main/scala/io/joern/scanners/c/Metrics.scala b/src/main/scala/io/joern/scanners/c/Metrics.scala index 6fb9166..d02d912 100644 --- a/src/main/scala/io/joern/scanners/c/Metrics.scala +++ b/src/main/scala/io/joern/scanners/c/Metrics.scala @@ -9,7 +9,7 @@ object Metrics extends QueryBundle { @q def tooManyParameters(n: Int = 4): Query = - queryInit( + Query.make( "too-many-params", Crew.fabs, s"Number of parameters larger than $n", @@ -22,7 +22,7 @@ object Metrics extends QueryBundle { @q def tooHighComplexity(n: Int = 4): Query = - queryInit( + Query.make( "too-high-complexity", Crew.fabs, s"Cyclomatic complexity higher than $n", @@ -35,7 +35,7 @@ object Metrics extends QueryBundle { @q def tooLong(n: Int = 1000): Query = - queryInit( + Query.make( "too-long", Crew.fabs, s"More than $n lines", @@ -48,7 +48,7 @@ object Metrics extends QueryBundle { @q def multipleReturns(): Query = - queryInit( + Query.make( "multiple-returns", Crew.fabs, s"Multiple returns", @@ -61,7 +61,7 @@ object Metrics extends QueryBundle { @q def tooManyLoops(n: Int = 4): Query = - queryInit( + Query.make( "too-many-loops", Crew.fabs, s"More than $n loops", @@ -78,7 +78,7 @@ object Metrics extends QueryBundle { @q def tooNested(n: Int = 3): Query = - queryInit( + Query.make( "too-nested", Crew.fabs, s"Nesting level higher than $n", diff --git a/src/main/scala/io/joern/scanners/c/NullTermination.scala b/src/main/scala/io/joern/scanners/c/NullTermination.scala index 2de7114..ade80cc 100644 --- a/src/main/scala/io/joern/scanners/c/NullTermination.scala +++ b/src/main/scala/io/joern/scanners/c/NullTermination.scala @@ -13,7 +13,7 @@ object NullTermination extends QueryBundle { @q def strncpyNoNullTerm()(implicit engineContext: EngineContext): Query = - queryInit( + Query.make( "strncpy-no-null-term", Crew.fabs, "strncpy is used and no null termination is nearby", diff --git a/src/main/scala/io/joern/scanners/c/RetvalChecks.scala b/src/main/scala/io/joern/scanners/c/RetvalChecks.scala index bf8b2dd..76bd4fd 100644 --- a/src/main/scala/io/joern/scanners/c/RetvalChecks.scala +++ b/src/main/scala/io/joern/scanners/c/RetvalChecks.scala @@ -9,7 +9,7 @@ object RetvalChecks extends QueryBundle { @q def uncheckedReadRecvMalloc(): Query = - queryInit( + Query.make( "unchecked-read-recv-malloc", Crew.fabs, "Unchecked read/recv/malloc", diff --git a/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala b/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala index a973367..03e417e 100644 --- a/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala +++ b/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala @@ -10,7 +10,7 @@ object SignedLeftShift extends QueryBundle { @q def signedLeftShift(): Query = - queryInit( + Query.make( "signed-left-shift", Crew.malte, "Signed Shift May Cause Undefined Behavior", diff --git a/src/main/scala/io/joern/scanners/c/UseAfterFree.scala b/src/main/scala/io/joern/scanners/c/UseAfterFree.scala index 0558bfc..a04abbc 100644 --- a/src/main/scala/io/joern/scanners/c/UseAfterFree.scala +++ b/src/main/scala/io/joern/scanners/c/UseAfterFree.scala @@ -15,7 +15,7 @@ object UseAfterFree extends QueryBundle { @q def freeFieldNoReassign()(implicit context: EngineContext): Query = - queryInit( + Query.make( "free-field-no-reassign", Crew.fabs, "A field of a parameter is free'd and not reassigned on all paths", @@ -56,7 +56,7 @@ object UseAfterFree extends QueryBundle { @q def freeReturnedValue()(implicit context: EngineContext): Query = - queryInit( + Query.make( "free-returned-value", Crew.malte, "A value that is returned through a parameter is free'd in a path", @@ -116,7 +116,7 @@ object UseAfterFree extends QueryBundle { @q def freePostDominatesUsage()(implicit context: EngineContext): Query = - queryInit( + Query.make( "free-follows-value-reuse", Crew.malte, "A value that is free'd is reused without reassignment.", diff --git a/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala b/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala index 23c6958..d337a0e 100644 --- a/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala +++ b/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala @@ -11,7 +11,7 @@ object DangerousFunctions extends QueryBundle { @q def execUsed(): Query = - queryInit( + Query.make( "call-to-exec", Crew.niko, "Dangerous function 'java.lang.Runtime.exec:java.lang.Process(java.lang.String)' used", From 84d3c5b5a4a23c776ae1261f653de19c7c6f09ed Mon Sep 17 00:00:00 2001 From: Claudiu-Vlad Ursache Date: Thu, 25 Mar 2021 12:17:30 +0100 Subject: [PATCH 2/4] Update README --- README.md | 39 ++++---- .../scala/io/joern/scanners/c/CopyLoops.scala | 13 ++- .../io/joern/scanners/c/CredentialDrop.scala | 28 +++--- .../joern/scanners/c/DangerousFunctions.scala | 98 +++++++++---------- .../joern/scanners/c/HeapBasedOverflow.scala | 14 +-- .../scala/io/joern/scanners/c/Metrics.scala | 84 ++++++++-------- .../io/joern/scanners/c/NullTermination.scala | 14 +-- .../io/joern/scanners/c/RetvalChecks.scala | 13 ++- .../io/joern/scanners/c/SignedLeftShift.scala | 13 ++- .../io/joern/scanners/c/UseAfterFree.scala | 42 ++++---- .../scanners/java/DangerousFunctions.scala | 14 +-- 11 files changed, 187 insertions(+), 185 deletions(-) diff --git a/README.md b/README.md index 0e8be06..4927065 100644 --- a/README.md +++ b/README.md @@ -64,25 +64,30 @@ as an example: object Metrics extends QueryBundle { @q - def tooManyParameters(n: Int = 4): Query = Query( - name = "too-many-parameters", - title = s"Number of parameters larger than $n", - description = - s"This query identifies functions with more than $n formal parameters", - score = 2.0, traversal = { cpg => - cpg.method.filter(_.parameter.size > n) - } - ) + def tooManyParameters(n: Int = 4): Query = + Query.make( + name = "too-many-params", + author = Crew.fabs, + title = s"Number of parameters larger than $n", + description = s"This query identifies functions with more than $n formal parameters", + score = 1.0, withStrRep({ cpg => + cpg.method.internal.filter(_.parameter.size > n) + }), + tags = List(QueryTags.metrics) + ) @q - def tooHighComplexity(n: Int = 4): Query = Query( - title = s"Cyclomatic complexity higher than $n", - description = - s"This query identifies functions with a cyclomatic complexity higher than $n", - score = 2.0, traversal = { cpg => - cpg.method.filter(_.controlStructure.size > n) - } - ) + def tooHighComplexity(n: Int = 4): Query = + Query.make( + name = "too-high-complexity", + author = Crew.fabs, + title = s"Cyclomatic complexity higher than $n", + description = s"This query identifies functions with a cyclomatic complexity higher than $n", + score = 1.0, withStrRep({ cpg => + cpg.method.internal.filter(_.controlStructure.size > n) + }), + tags = List(QueryTags.metrics) + ) ... } ``` diff --git a/src/main/scala/io/joern/scanners/c/CopyLoops.scala b/src/main/scala/io/joern/scanners/c/CopyLoops.scala index b78ab63..46ef997 100644 --- a/src/main/scala/io/joern/scanners/c/CopyLoops.scala +++ b/src/main/scala/io/joern/scanners/c/CopyLoops.scala @@ -10,16 +10,16 @@ object CopyLoops extends QueryBundle { @q def isCopyLoop(): Query = Query.make( - "copy-loop", - Crew.fabs, - "Copy loop detected", - """ + name = "copy-loop", + author = Crew.fabs, + title = "Copy loop detected", + description = """ |For (buf, indices) pairs, determine those inside control structures (for, while, if ...) |where any of the calls made outside of the body (block) are Inc operations. Determine |the first argument of that Inc operation and check if they are used as indices for |the write operation into the buffer. |""".stripMargin, - 2, { cpg => + score = 2, withStrRep({ cpg => cpg.assignment.target.isArrayAccess .map { access => (access.array, access.subscripts.code.toSet) @@ -35,8 +35,7 @@ object CopyLoops extends QueryBundle { (incIdentifiers & subscripts).nonEmpty } .map(_._1) - }, - List() + }), ) } diff --git a/src/main/scala/io/joern/scanners/c/CredentialDrop.scala b/src/main/scala/io/joern/scanners/c/CredentialDrop.scala index 3098ae0..ed44f87 100644 --- a/src/main/scala/io/joern/scanners/c/CredentialDrop.scala +++ b/src/main/scala/io/joern/scanners/c/CredentialDrop.scala @@ -12,43 +12,43 @@ object CredentialDrop extends QueryBundle { @q def userCredDrop(): Query = Query.make( - "setuid-without-setgid", - Crew.malte, - "Process user ID is changed without changing groups first", - """ + name = "setuid-without-setgid", + author = Crew.malte, + title = "Process user ID is changed without changing groups first", + description = """ |The set*uid system calls do not affect the groups a process belongs to. However, often |there exists a group that is equivalent to a user (e.g. wheel or shadow groups are often |equivalent to the root user). |Group membership can only be changed by the root user. |Changes to the user should therefore always be preceded by calls to set*gid and setgroups, |""".stripMargin, - 2, { cpg => + score = 2, withStrRep({ cpg => cpg .method("set(res|re|e|)uid") .callIn .whereNot(_.dominatedBy.isCall.name("set(res|re|e|)?gid")) - }, - List(QueryTags.setxid) + }), + tags = List(QueryTags.setxid) ) @q def groupCredDrop(): Query = Query.make( - "setgid-without-setgroups", - Crew.malte, - "Process group membership is changed without setting ancillary groups first", - """ + name = "setgid-without-setgroups", + author = Crew.malte, + title = "Process group membership is changed without setting ancillary groups first", + description = """ |The set*gid system calls do not affect the ancillary groups a process belongs to. |Changes to the group membership should therefore always be preceded by a call to setgroups. |Otherwise the process may still be a secondary member of the group it tries to disavow. |""".stripMargin, - 2, { cpg => + score = 2, withStrRep({ cpg => cpg .method("set(res|re|e|)gid") .callIn .whereNot(_.dominatedBy.isCall.name("setgroups")) - }, - List(QueryTags.setxid) + }), + tags = List(QueryTags.setxid) ) } diff --git a/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala b/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala index 55b4395..b8fc444 100644 --- a/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala +++ b/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala @@ -12,32 +12,32 @@ object DangerousFunctions extends QueryBundle { @q def getsUsed(): Query = Query.make( - "call-to-gets", - Crew.suchakra, - "Dangerous function gets() used", - """ + name = "call-to-gets", + author = Crew.suchakra, + title = "Dangerous function gets() used", + description = """ | Avoid `gets` function as it can lead to reads beyond buffer | boundary and cause | buffer overflows. Some secure alternatives are `fgets` and `gets_s`. |""".stripMargin, - 8, { cpg => + score = 8, withStrRep({ cpg => cpg.method("gets").callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) @q def argvUsedInPrintf(): Query = Query.make( - "format-controlled-printf", - Crew.suchakra, - "Non-constant format string passed to printf/sprintf/vsprintf", - """ + name = "format-controlled-printf", + author = Crew.suchakra, + title = "Non-constant format string passed to printf/sprintf/vsprintf", + description = """ | Avoid user controlled format strings like "argv" in printf, sprintf and vsprintf | functions as they can cause memory corruption. Some secure | alternatives are `snprintf` and `vsnprintf`. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => cpg .method("printf") .callIn @@ -46,94 +46,94 @@ object DangerousFunctions extends QueryBundle { .method("(sprintf|vsprintf)") .callIn .whereNot(_.argument.order(2).isLiteral) - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) @q def scanfUsed(): Query = Query.make( - "call-to-scanf", - Crew.suchakra, - "Insecure function scanf() used", - """ + name = "call-to-scanf", + author = Crew.suchakra, + title = "Insecure function scanf() used", + description = """ | Avoid `scanf` function as it can lead to reads beyond buffer | boundary and cause buffer overflows. A secure alternative is `fgets`. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => cpg.method("scanf").callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) @q def strcatUsed(): Query = Query.make( - "call-to-strcat", - Crew.suchakra, - "Dangerous functions `strcat` or `strncat` used", - """ + name = "call-to-strcat", + author = Crew.suchakra, + title = "Dangerous functions `strcat` or `strncat` used", + description = """ | Avoid `strcat` or `strncat` functions. These can be used insecurely | causing non null-termianted strings leading to memory corruption. | A secure alternative is `strcat_s`. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => cpg.method("(strcat|strncat)").callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) @q def strcpyUsed(): Query = Query.make( - "call-to-strcpy", - Crew.suchakra, - "Dangerous functions `strcpy` or `strncpy` used", - """ + name = "call-to-strcpy", + author = Crew.suchakra, + title = "Dangerous functions `strcpy` or `strncpy` used", + description = """ | Avoid `strcpy` or `strncpy` function. `strcpy` does not check buffer | lengths. | A possible mitigation could be `strncpy` which could prevent | buffer overflows but does not null-terminate strings leading to | memory corruption. A secure alternative (on BSD) is `strlcpy`. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => cpg.method("(strcpy|strncpy)").callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) @q def strtokUsed(): Query = Query.make( - "call-to-strtok", - Crew.suchakra, - "Dangerous function strtok() used", - """ + name = "call-to-strtok", + author = Crew.suchakra, + title = "Dangerous function strtok() used", + description = """ | Avoid `strtok` function as it modifies the original string in place | and appends a null character after each token. This makes the | original string unsafe. Suggested alternative is `strtok_r` with | `saveptr`. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => cpg.method("strtok").callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) @q def getwdUsed(): Query = Query.make( - "call-to-getwd", - Crew.claudiu, - "Dangerous function getwd() used", - """ + name = "call-to-getwd", + author = Crew.claudiu, + title = "Dangerous function getwd() used", + description = """ | Avoid the `getwd` function, it does not check buffer lengths. | Use `getcwd` instead, as it checks the buffer size. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => cpg.method("getwd").callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) } diff --git a/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala b/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala index 063d9ab..0473df3 100644 --- a/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala +++ b/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala @@ -21,11 +21,11 @@ object HeapBasedOverflow extends QueryBundle { @q def mallocMemcpyIntOverflow()(implicit context: EngineContext): Query = Query.make( - "malloc-memcpy-int-overflow", - Crew.fabs, - "Dangerous copy-operation into heap-allocated buffer", - "-", - 4, { cpg => + name = "malloc-memcpy-int-overflow", + author = Crew.fabs, + title = "Dangerous copy-operation into heap-allocated buffer", + description = "-", + score = 4, withStrRep({ cpg => val src = cpg .method(".*malloc$") .callIn @@ -45,8 +45,8 @@ object HeapBasedOverflow extends QueryBundle { .whereNot(_.argument(1).codeExact(memcpyCall.argument(3).code)) .hasNext } - }, - List(QueryTags.integers) + }), + tags = List(QueryTags.integers) ) } diff --git a/src/main/scala/io/joern/scanners/c/Metrics.scala b/src/main/scala/io/joern/scanners/c/Metrics.scala index d02d912..34c5808 100644 --- a/src/main/scala/io/joern/scanners/c/Metrics.scala +++ b/src/main/scala/io/joern/scanners/c/Metrics.scala @@ -10,83 +10,83 @@ object Metrics extends QueryBundle { @q def tooManyParameters(n: Int = 4): Query = Query.make( - "too-many-params", - Crew.fabs, - s"Number of parameters larger than $n", - s"This query identifies functions with more than $n formal parameters", - 1.0, { cpg => + name = "too-many-params", + author = Crew.fabs, + title = s"Number of parameters larger than $n", + description = s"This query identifies functions with more than $n formal parameters", + score = 1.0, withStrRep({ cpg => cpg.method.internal.filter(_.parameter.size > n) - }, - List(QueryTags.metrics) + }), + tags = List(QueryTags.metrics) ) @q def tooHighComplexity(n: Int = 4): Query = Query.make( - "too-high-complexity", - Crew.fabs, - s"Cyclomatic complexity higher than $n", - s"This query identifies functions with a cyclomatic complexity higher than $n", - 1.0, { cpg => + name = "too-high-complexity", + author = Crew.fabs, + title = s"Cyclomatic complexity higher than $n", + description = s"This query identifies functions with a cyclomatic complexity higher than $n", + score = 1.0, withStrRep({ cpg => cpg.method.internal.filter(_.controlStructure.size > n) - }, - List(QueryTags.metrics) + }), + tags = List(QueryTags.metrics) ) @q def tooLong(n: Int = 1000): Query = Query.make( - "too-long", - Crew.fabs, - s"More than $n lines", - s"This query identifies functions that are more than $n lines long", - 1.0, { cpg => + name = "too-long", + author = Crew.fabs, + title = s"More than $n lines", + description = s"This query identifies functions that are more than $n lines long", + score = 1.0, withStrRep({ cpg => cpg.method.internal.filter(_.numberOfLines > n) - }, - List(QueryTags.metrics) + }), + tags = List(QueryTags.metrics) ) @q def multipleReturns(): Query = Query.make( - "multiple-returns", - Crew.fabs, - s"Multiple returns", - "This query identifies functions with more than one return", - 1.0, { cpg => + name = "multiple-returns", + author = Crew.fabs, + title = s"Multiple returns", + description = "This query identifies functions with more than one return", + score = 1.0, withStrRep({ cpg => cpg.method.internal.filter(_.ast.isReturn.l.size > 1) - }, - List(QueryTags.metrics) + }), + tags = List(QueryTags.metrics) ) @q def tooManyLoops(n: Int = 4): Query = Query.make( - "too-many-loops", - Crew.fabs, - s"More than $n loops", - s"This query identifies functions with more than $n loops", - 1.0, { cpg => + name = "too-many-loops", + author = Crew.fabs, + title = s"More than $n loops", + description = s"This query identifies functions with more than $n loops", + score = 1.0, withStrRep({ cpg => cpg.method.internal .filter( _.ast.isControlStructure .parserTypeName("(For|Do|While).*") .size > n) - }, - List(QueryTags.metrics) + }), + tags = List(QueryTags.metrics) ) @q def tooNested(n: Int = 3): Query = Query.make( - "too-nested", - Crew.fabs, - s"Nesting level higher than $n", - s"This query identifies functions with a nesting level higher than $n", - 1.0, { cpg => + name = "too-nested", + author = Crew.fabs, + title = s"Nesting level higher than $n", + description = s"This query identifies functions with a nesting level higher than $n", + score = 1.0, withStrRep({ cpg => cpg.method.internal.filter(_.depth(_.isControlStructure) > n) - }, - List(QueryTags.metrics) + }), + tags = List(QueryTags.metrics) ) } diff --git a/src/main/scala/io/joern/scanners/c/NullTermination.scala b/src/main/scala/io/joern/scanners/c/NullTermination.scala index ade80cc..b569e3f 100644 --- a/src/main/scala/io/joern/scanners/c/NullTermination.scala +++ b/src/main/scala/io/joern/scanners/c/NullTermination.scala @@ -14,10 +14,10 @@ object NullTermination extends QueryBundle { @q def strncpyNoNullTerm()(implicit engineContext: EngineContext): Query = Query.make( - "strncpy-no-null-term", - Crew.fabs, - "strncpy is used and no null termination is nearby", - """ + name = "strncpy-no-null-term", + author = Crew.fabs, + title = "strncpy is used and no null termination is nearby", + description = """ | Upon calling `strncpy` with a source string that is larger | than the destination buffer, the destination buffer is not | null-terminated by `strncpy` and there is no explicit @@ -25,7 +25,7 @@ object NullTermination extends QueryBundle { | buffer size is at least 1 larger than the size passed | to `strncpy`. |""".stripMargin, - 4, { cpg => + score = 4, withStrRep({ cpg => val allocations = cpg.method(".*malloc$").callIn.argument(1).l cpg .method("strncpy") @@ -44,8 +44,8 @@ object NullTermination extends QueryBundle { .isEmpty } .map(_._2) - }, - List(QueryTags.strings) + }), + tags = List(QueryTags.strings) ) } diff --git a/src/main/scala/io/joern/scanners/c/RetvalChecks.scala b/src/main/scala/io/joern/scanners/c/RetvalChecks.scala index 76bd4fd..3800943 100644 --- a/src/main/scala/io/joern/scanners/c/RetvalChecks.scala +++ b/src/main/scala/io/joern/scanners/c/RetvalChecks.scala @@ -10,15 +10,15 @@ object RetvalChecks extends QueryBundle { @q def uncheckedReadRecvMalloc(): Query = Query.make( - "unchecked-read-recv-malloc", - Crew.fabs, - "Unchecked read/recv/malloc", - """ + name = "unchecked-read-recv-malloc", + author = Crew.fabs, + title = "Unchecked read/recv/malloc", + description = """ |The return value of a read/recv/malloc call is not checked directly and |the variable it has been assigned to (if any) does not |occur in any check within the caller. |""".stripMargin, - 3.0, { cpg => + score = 3.0, withStrRep({ cpg => implicit val noResolve: NoResolve.type = NoResolve val callsNotDirectlyChecked = cpg .method("(read|recv|malloc)") @@ -37,8 +37,7 @@ object RetvalChecks extends QueryBundle { val targets = call.inAssignment.target.code.toSet (targets & checkedVars).nonEmpty } - }, - List() + }), ) } diff --git a/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala b/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala index 03e417e..2e26933 100644 --- a/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala +++ b/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala @@ -11,20 +11,19 @@ object SignedLeftShift extends QueryBundle { @q def signedLeftShift(): Query = Query.make( - "signed-left-shift", - Crew.malte, - "Signed Shift May Cause Undefined Behavior", - """ + name = "signed-left-shift", + author = Crew.malte, + title = "Signed Shift May Cause Undefined Behavior", + description = """ |Signed integer overflow is undefined behavior. Shifts of signed values to the |left are very prone to overflow. |""".stripMargin, - 2, { cpg => + score = 2, withStrRep({ cpg => cpg.call .nameExact(Operators.shiftLeft, Operators.assignmentShiftLeft) .where(_.argument(1).typ.fullNameExact("int", "long")) .filterNot(_.argument.isLiteral.size == 2) // assume such constant values produces a correct result - }, - List() + }), ) } diff --git a/src/main/scala/io/joern/scanners/c/UseAfterFree.scala b/src/main/scala/io/joern/scanners/c/UseAfterFree.scala index a04abbc..af89fef 100644 --- a/src/main/scala/io/joern/scanners/c/UseAfterFree.scala +++ b/src/main/scala/io/joern/scanners/c/UseAfterFree.scala @@ -16,10 +16,10 @@ object UseAfterFree extends QueryBundle { @q def freeFieldNoReassign()(implicit context: EngineContext): Query = Query.make( - "free-field-no-reassign", - Crew.fabs, - "A field of a parameter is free'd and not reassigned on all paths", - """ + name = "free-field-no-reassign", + author = Crew.fabs, + title = "A field of a parameter is free'd and not reassigned on all paths", + description = """ | The function is able to modify a field of a structure passed in by | the caller. It frees this field and does not guarantee that on | all paths to the exit, the field is reassigned. If any @@ -28,7 +28,7 @@ object UseAfterFree extends QueryBundle { | or clear the entire structure, as in that case, it is unlikely that the | passed in structure will be used again. |""".stripMargin, - 5.0, { cpg => + score = 5.0, withStrRep({ cpg => val freeOfStructField = cpg .method("free") .callIn @@ -50,17 +50,17 @@ object UseAfterFree extends QueryBundle { freeOfStructField.argument(1).filter { arg => arg.method.methodReturn.reachableBy(arg).nonEmpty } - }, - List(QueryTags.uaf) + }), + tags = List(QueryTags.uaf) ) @q def freeReturnedValue()(implicit context: EngineContext): Query = Query.make( - "free-returned-value", - Crew.malte, - "A value that is returned through a parameter is free'd in a path", - """ + name = "free-returned-value", + author = Crew.malte, + title = "A value that is returned through a parameter is free'd in a path", + description = """ |The function sets a field of a function parameter to a value of a local |variable. |This variable is then freed in some paths. Unless the value set in the @@ -69,7 +69,7 @@ object UseAfterFree extends QueryBundle { | |Finds bugs like CVE-2019-18902. |""".stripMargin, - 5.0, { cpg => + score = 5.0, withStrRep({ cpg => def outParams = cpg.parameter .typeFullName(".+\\*") @@ -110,23 +110,23 @@ object UseAfterFree extends QueryBundle { case (id, freeCall) => freeCall.dominatedBy.exists(_ == id) } .flatMap(_._1) - }, - List(QueryTags.uaf) + }), + tags = List(QueryTags.uaf) ) @q def freePostDominatesUsage()(implicit context: EngineContext): Query = Query.make( - "free-follows-value-reuse", - Crew.malte, - "A value that is free'd is reused without reassignment.", - """ + name = "free-follows-value-reuse", + author = Crew.malte, + title = "A value that is free'd is reused without reassignment.", + description = """ |A value is used after being free'd in a path that leads to it |without reassignment. | |Modeled after CVE-2019-18903. |""".stripMargin, - 5.0, { cpg => + score = 5.0, withStrRep({ cpg => cpg.method .name("(.*_)?free") .filter(_.parameter.size == 1) @@ -146,8 +146,8 @@ object UseAfterFree extends QueryBundle { .isIdentifier .codeExact(freedIdentifierCode) }) - }, - List(QueryTags.uaf) + }), + tags = List(QueryTags.uaf) ) } diff --git a/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala b/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala index d337a0e..98b5c7a 100644 --- a/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala +++ b/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala @@ -12,18 +12,18 @@ object DangerousFunctions extends QueryBundle { @q def execUsed(): Query = Query.make( - "call-to-exec", - Crew.niko, - "Dangerous function 'java.lang.Runtime.exec:java.lang.Process(java.lang.String)' used", - """ + name = "call-to-exec", + author = Crew.niko, + title = "Dangerous function 'java.lang.Runtime.exec:java.lang.Process(java.lang.String)' used", + description = """ | A call to the function `java.lang.Runtime.exec:java.lang.Process(java.lang.String)` | could result in a potential remote code execution. |""".stripMargin, - 8, { cpg => + score = 8, withStrRep({ cpg => cpg .method("java.lang.Runtime.exec") .callIn - }, - List(QueryTags.badfn) + }), + tags = List(QueryTags.badfn) ) } From 94f6d99b97e67bb596c27886e1f30ff8280f081d Mon Sep 17 00:00:00 2001 From: Claudiu-Vlad Ursache Date: Thu, 25 Mar 2021 12:23:37 +0100 Subject: [PATCH 3/4] Run `scalafmt` --- README.md | 6 ++- .../scala/io/joern/scanners/c/CopyLoops.scala | 6 ++- .../io/joern/scanners/c/CredentialDrop.scala | 15 ++++--- .../joern/scanners/c/DangerousFunctions.scala | 42 ++++++++++++------- .../joern/scanners/c/HeapBasedOverflow.scala | 3 +- .../joern/scanners/c/IntegerTruncations.scala | 6 ++- .../scala/io/joern/scanners/c/Metrics.scala | 30 ++++++++----- .../io/joern/scanners/c/NullTermination.scala | 3 +- .../io/joern/scanners/c/RetvalChecks.scala | 6 ++- .../io/joern/scanners/c/SignedLeftShift.scala | 6 ++- .../io/joern/scanners/c/UseAfterFree.scala | 18 +++++--- .../scanners/java/DangerousFunctions.scala | 9 ++-- 12 files changed, 100 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 4927065..a1fc765 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,8 @@ object Metrics extends QueryBundle { author = Crew.fabs, title = s"Number of parameters larger than $n", description = s"This query identifies functions with more than $n formal parameters", - score = 1.0, withStrRep({ cpg => + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.parameter.size > n) }), tags = List(QueryTags.metrics) @@ -83,7 +84,8 @@ object Metrics extends QueryBundle { author = Crew.fabs, title = s"Cyclomatic complexity higher than $n", description = s"This query identifies functions with a cyclomatic complexity higher than $n", - score = 1.0, withStrRep({ cpg => + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.controlStructure.size > n) }), tags = List(QueryTags.metrics) diff --git a/src/main/scala/io/joern/scanners/c/CopyLoops.scala b/src/main/scala/io/joern/scanners/c/CopyLoops.scala index 46ef997..3c45f9e 100644 --- a/src/main/scala/io/joern/scanners/c/CopyLoops.scala +++ b/src/main/scala/io/joern/scanners/c/CopyLoops.scala @@ -13,13 +13,15 @@ object CopyLoops extends QueryBundle { name = "copy-loop", author = Crew.fabs, title = "Copy loop detected", - description = """ + description = + """ |For (buf, indices) pairs, determine those inside control structures (for, while, if ...) |where any of the calls made outside of the body (block) are Inc operations. Determine |the first argument of that Inc operation and check if they are used as indices for |the write operation into the buffer. |""".stripMargin, - score = 2, withStrRep({ cpg => + score = 2, + withStrRep({ cpg => cpg.assignment.target.isArrayAccess .map { access => (access.array, access.subscripts.code.toSet) diff --git a/src/main/scala/io/joern/scanners/c/CredentialDrop.scala b/src/main/scala/io/joern/scanners/c/CredentialDrop.scala index ed44f87..38c65ab 100644 --- a/src/main/scala/io/joern/scanners/c/CredentialDrop.scala +++ b/src/main/scala/io/joern/scanners/c/CredentialDrop.scala @@ -15,14 +15,16 @@ object CredentialDrop extends QueryBundle { name = "setuid-without-setgid", author = Crew.malte, title = "Process user ID is changed without changing groups first", - description = """ + description = + """ |The set*uid system calls do not affect the groups a process belongs to. However, often |there exists a group that is equivalent to a user (e.g. wheel or shadow groups are often |equivalent to the root user). |Group membership can only be changed by the root user. |Changes to the user should therefore always be preceded by calls to set*gid and setgroups, |""".stripMargin, - score = 2, withStrRep({ cpg => + score = 2, + withStrRep({ cpg => cpg .method("set(res|re|e|)uid") .callIn @@ -36,13 +38,16 @@ object CredentialDrop extends QueryBundle { Query.make( name = "setgid-without-setgroups", author = Crew.malte, - title = "Process group membership is changed without setting ancillary groups first", - description = """ + title = + "Process group membership is changed without setting ancillary groups first", + description = + """ |The set*gid system calls do not affect the ancillary groups a process belongs to. |Changes to the group membership should therefore always be preceded by a call to setgroups. |Otherwise the process may still be a secondary member of the group it tries to disavow. |""".stripMargin, - score = 2, withStrRep({ cpg => + score = 2, + withStrRep({ cpg => cpg .method("set(res|re|e|)gid") .callIn diff --git a/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala b/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala index b8fc444..cf285f3 100644 --- a/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala +++ b/src/main/scala/io/joern/scanners/c/DangerousFunctions.scala @@ -15,12 +15,14 @@ object DangerousFunctions extends QueryBundle { name = "call-to-gets", author = Crew.suchakra, title = "Dangerous function gets() used", - description = """ + description = + """ | Avoid `gets` function as it can lead to reads beyond buffer | boundary and cause | buffer overflows. Some secure alternatives are `fgets` and `gets_s`. |""".stripMargin, - score = 8, withStrRep({ cpg => + score = 8, + withStrRep({ cpg => cpg.method("gets").callIn }), tags = List(QueryTags.badfn) @@ -32,12 +34,14 @@ object DangerousFunctions extends QueryBundle { name = "format-controlled-printf", author = Crew.suchakra, title = "Non-constant format string passed to printf/sprintf/vsprintf", - description = """ + description = + """ | Avoid user controlled format strings like "argv" in printf, sprintf and vsprintf | functions as they can cause memory corruption. Some secure | alternatives are `snprintf` and `vsnprintf`. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => cpg .method("printf") .callIn @@ -56,11 +60,13 @@ object DangerousFunctions extends QueryBundle { name = "call-to-scanf", author = Crew.suchakra, title = "Insecure function scanf() used", - description = """ + description = + """ | Avoid `scanf` function as it can lead to reads beyond buffer | boundary and cause buffer overflows. A secure alternative is `fgets`. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => cpg.method("scanf").callIn }), tags = List(QueryTags.badfn) @@ -72,12 +78,14 @@ object DangerousFunctions extends QueryBundle { name = "call-to-strcat", author = Crew.suchakra, title = "Dangerous functions `strcat` or `strncat` used", - description = """ + description = + """ | Avoid `strcat` or `strncat` functions. These can be used insecurely | causing non null-termianted strings leading to memory corruption. | A secure alternative is `strcat_s`. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => cpg.method("(strcat|strncat)").callIn }), tags = List(QueryTags.badfn) @@ -89,14 +97,16 @@ object DangerousFunctions extends QueryBundle { name = "call-to-strcpy", author = Crew.suchakra, title = "Dangerous functions `strcpy` or `strncpy` used", - description = """ + description = + """ | Avoid `strcpy` or `strncpy` function. `strcpy` does not check buffer | lengths. | A possible mitigation could be `strncpy` which could prevent | buffer overflows but does not null-terminate strings leading to | memory corruption. A secure alternative (on BSD) is `strlcpy`. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => cpg.method("(strcpy|strncpy)").callIn }), tags = List(QueryTags.badfn) @@ -108,13 +118,15 @@ object DangerousFunctions extends QueryBundle { name = "call-to-strtok", author = Crew.suchakra, title = "Dangerous function strtok() used", - description = """ + description = + """ | Avoid `strtok` function as it modifies the original string in place | and appends a null character after each token. This makes the | original string unsafe. Suggested alternative is `strtok_r` with | `saveptr`. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => cpg.method("strtok").callIn }), tags = List(QueryTags.badfn) @@ -126,11 +138,13 @@ object DangerousFunctions extends QueryBundle { name = "call-to-getwd", author = Crew.claudiu, title = "Dangerous function getwd() used", - description = """ + description = + """ | Avoid the `getwd` function, it does not check buffer lengths. | Use `getcwd` instead, as it checks the buffer size. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => cpg.method("getwd").callIn }), tags = List(QueryTags.badfn) diff --git a/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala b/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala index 0473df3..7a4b3a4 100644 --- a/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala +++ b/src/main/scala/io/joern/scanners/c/HeapBasedOverflow.scala @@ -25,7 +25,8 @@ object HeapBasedOverflow extends QueryBundle { author = Crew.fabs, title = "Dangerous copy-operation into heap-allocated buffer", description = "-", - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => val src = cpg .method(".*malloc$") .callIn diff --git a/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala b/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala index ac1cf34..8af562b 100644 --- a/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala +++ b/src/main/scala/io/joern/scanners/c/IntegerTruncations.scala @@ -20,13 +20,15 @@ object IntegerTruncations extends QueryBundle { name = "strlen-truncation", author = Crew.fabs, title = "Truncation in assignment involving `strlen` call", - description = """ + description = + """ |The return value of `strlen` is stored in a variable that is known |to be of type `int` as opposed to `size_t`. `int` is only 32 bit |wide on many 64 bit platforms, and thus, this may result in a |truncation. |""".stripMargin, - score = 2, withStrRep({ cpg => + score = 2, + withStrRep({ cpg => cpg .method("strlen") .callIn diff --git a/src/main/scala/io/joern/scanners/c/Metrics.scala b/src/main/scala/io/joern/scanners/c/Metrics.scala index 34c5808..0c00a56 100644 --- a/src/main/scala/io/joern/scanners/c/Metrics.scala +++ b/src/main/scala/io/joern/scanners/c/Metrics.scala @@ -13,8 +13,10 @@ object Metrics extends QueryBundle { name = "too-many-params", author = Crew.fabs, title = s"Number of parameters larger than $n", - description = s"This query identifies functions with more than $n formal parameters", - score = 1.0, withStrRep({ cpg => + description = + s"This query identifies functions with more than $n formal parameters", + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.parameter.size > n) }), tags = List(QueryTags.metrics) @@ -26,8 +28,10 @@ object Metrics extends QueryBundle { name = "too-high-complexity", author = Crew.fabs, title = s"Cyclomatic complexity higher than $n", - description = s"This query identifies functions with a cyclomatic complexity higher than $n", - score = 1.0, withStrRep({ cpg => + description = + s"This query identifies functions with a cyclomatic complexity higher than $n", + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.controlStructure.size > n) }), tags = List(QueryTags.metrics) @@ -39,8 +43,10 @@ object Metrics extends QueryBundle { name = "too-long", author = Crew.fabs, title = s"More than $n lines", - description = s"This query identifies functions that are more than $n lines long", - score = 1.0, withStrRep({ cpg => + description = + s"This query identifies functions that are more than $n lines long", + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.numberOfLines > n) }), tags = List(QueryTags.metrics) @@ -53,7 +59,8 @@ object Metrics extends QueryBundle { author = Crew.fabs, title = s"Multiple returns", description = "This query identifies functions with more than one return", - score = 1.0, withStrRep({ cpg => + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.ast.isReturn.l.size > 1) }), tags = List(QueryTags.metrics) @@ -66,7 +73,8 @@ object Metrics extends QueryBundle { author = Crew.fabs, title = s"More than $n loops", description = s"This query identifies functions with more than $n loops", - score = 1.0, withStrRep({ cpg => + score = 1.0, + withStrRep({ cpg => cpg.method.internal .filter( _.ast.isControlStructure @@ -82,8 +90,10 @@ object Metrics extends QueryBundle { name = "too-nested", author = Crew.fabs, title = s"Nesting level higher than $n", - description = s"This query identifies functions with a nesting level higher than $n", - score = 1.0, withStrRep({ cpg => + description = + s"This query identifies functions with a nesting level higher than $n", + score = 1.0, + withStrRep({ cpg => cpg.method.internal.filter(_.depth(_.isControlStructure) > n) }), tags = List(QueryTags.metrics) diff --git a/src/main/scala/io/joern/scanners/c/NullTermination.scala b/src/main/scala/io/joern/scanners/c/NullTermination.scala index b569e3f..2303576 100644 --- a/src/main/scala/io/joern/scanners/c/NullTermination.scala +++ b/src/main/scala/io/joern/scanners/c/NullTermination.scala @@ -25,7 +25,8 @@ object NullTermination extends QueryBundle { | buffer size is at least 1 larger than the size passed | to `strncpy`. |""".stripMargin, - score = 4, withStrRep({ cpg => + score = 4, + withStrRep({ cpg => val allocations = cpg.method(".*malloc$").callIn.argument(1).l cpg .method("strncpy") diff --git a/src/main/scala/io/joern/scanners/c/RetvalChecks.scala b/src/main/scala/io/joern/scanners/c/RetvalChecks.scala index 3800943..47ebbb0 100644 --- a/src/main/scala/io/joern/scanners/c/RetvalChecks.scala +++ b/src/main/scala/io/joern/scanners/c/RetvalChecks.scala @@ -13,12 +13,14 @@ object RetvalChecks extends QueryBundle { name = "unchecked-read-recv-malloc", author = Crew.fabs, title = "Unchecked read/recv/malloc", - description = """ + description = + """ |The return value of a read/recv/malloc call is not checked directly and |the variable it has been assigned to (if any) does not |occur in any check within the caller. |""".stripMargin, - score = 3.0, withStrRep({ cpg => + score = 3.0, + withStrRep({ cpg => implicit val noResolve: NoResolve.type = NoResolve val callsNotDirectlyChecked = cpg .method("(read|recv|malloc)") diff --git a/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala b/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala index 2e26933..6242ef4 100644 --- a/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala +++ b/src/main/scala/io/joern/scanners/c/SignedLeftShift.scala @@ -14,11 +14,13 @@ object SignedLeftShift extends QueryBundle { name = "signed-left-shift", author = Crew.malte, title = "Signed Shift May Cause Undefined Behavior", - description = """ + description = + """ |Signed integer overflow is undefined behavior. Shifts of signed values to the |left are very prone to overflow. |""".stripMargin, - score = 2, withStrRep({ cpg => + score = 2, + withStrRep({ cpg => cpg.call .nameExact(Operators.shiftLeft, Operators.assignmentShiftLeft) .where(_.argument(1).typ.fullNameExact("int", "long")) diff --git a/src/main/scala/io/joern/scanners/c/UseAfterFree.scala b/src/main/scala/io/joern/scanners/c/UseAfterFree.scala index af89fef..ee1cab1 100644 --- a/src/main/scala/io/joern/scanners/c/UseAfterFree.scala +++ b/src/main/scala/io/joern/scanners/c/UseAfterFree.scala @@ -19,7 +19,8 @@ object UseAfterFree extends QueryBundle { name = "free-field-no-reassign", author = Crew.fabs, title = "A field of a parameter is free'd and not reassigned on all paths", - description = """ + description = + """ | The function is able to modify a field of a structure passed in by | the caller. It frees this field and does not guarantee that on | all paths to the exit, the field is reassigned. If any @@ -28,7 +29,8 @@ object UseAfterFree extends QueryBundle { | or clear the entire structure, as in that case, it is unlikely that the | passed in structure will be used again. |""".stripMargin, - score = 5.0, withStrRep({ cpg => + score = 5.0, + withStrRep({ cpg => val freeOfStructField = cpg .method("free") .callIn @@ -60,7 +62,8 @@ object UseAfterFree extends QueryBundle { name = "free-returned-value", author = Crew.malte, title = "A value that is returned through a parameter is free'd in a path", - description = """ + description = + """ |The function sets a field of a function parameter to a value of a local |variable. |This variable is then freed in some paths. Unless the value set in the @@ -69,7 +72,8 @@ object UseAfterFree extends QueryBundle { | |Finds bugs like CVE-2019-18902. |""".stripMargin, - score = 5.0, withStrRep({ cpg => + score = 5.0, + withStrRep({ cpg => def outParams = cpg.parameter .typeFullName(".+\\*") @@ -120,13 +124,15 @@ object UseAfterFree extends QueryBundle { name = "free-follows-value-reuse", author = Crew.malte, title = "A value that is free'd is reused without reassignment.", - description = """ + description = + """ |A value is used after being free'd in a path that leads to it |without reassignment. | |Modeled after CVE-2019-18903. |""".stripMargin, - score = 5.0, withStrRep({ cpg => + score = 5.0, + withStrRep({ cpg => cpg.method .name("(.*_)?free") .filter(_.parameter.size == 1) diff --git a/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala b/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala index 98b5c7a..5d93108 100644 --- a/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala +++ b/src/main/scala/io/joern/scanners/java/DangerousFunctions.scala @@ -14,12 +14,15 @@ object DangerousFunctions extends QueryBundle { Query.make( name = "call-to-exec", author = Crew.niko, - title = "Dangerous function 'java.lang.Runtime.exec:java.lang.Process(java.lang.String)' used", - description = """ + title = + "Dangerous function 'java.lang.Runtime.exec:java.lang.Process(java.lang.String)' used", + description = + """ | A call to the function `java.lang.Runtime.exec:java.lang.Process(java.lang.String)` | could result in a potential remote code execution. |""".stripMargin, - score = 8, withStrRep({ cpg => + score = 8, + withStrRep({ cpg => cpg .method("java.lang.Runtime.exec") .callIn From 8d6a6573a7c141e7d3ca4601540b7b73d809be37 Mon Sep 17 00:00:00 2001 From: Claudiu-Vlad Ursache Date: Thu, 25 Mar 2021 12:48:18 +0100 Subject: [PATCH 4/4] Fix missing import --- src/main/scala/io/joern/scanners/c/NullTermination.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/io/joern/scanners/c/NullTermination.scala b/src/main/scala/io/joern/scanners/c/NullTermination.scala index 2303576..014bce2 100644 --- a/src/main/scala/io/joern/scanners/c/NullTermination.scala +++ b/src/main/scala/io/joern/scanners/c/NullTermination.scala @@ -3,7 +3,7 @@ package io.joern.scanners.c import io.joern.scanners.{Crew, QueryTags} import io.shiftleft.semanticcpg.language._ import io.shiftleft.dataflowengineoss.language._ -import io.shiftleft.console.{Query, QueryBundle, q} +import io.shiftleft.console.{Query, QueryBundle, q, TraversalWithStrRep} import io.shiftleft.dataflowengineoss.queryengine.EngineContext import io.shiftleft.macros.QueryMacros._